-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ambushed zone #80
Ambushed zone #80
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool zone!
I'm unsure about the reward modification as a whole here though. Seeing only base game draw cards in the rewards seems very one note and only seeing draw potions aso doesn't feel great.
What about adding a single draw card as an additional card reward that players may pick in addition to the normal card reward?
https://github.com/erasels/SpireMapOverhaul/blob/main/src/main/java/spireMapOverhaul/rewards/SingleCardReward.java
That may also help justify the harder than normal fights.
} | ||
} | ||
|
||
private static class Locator extends SpireInsertLocator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it not just a prefix patch? Or an rloc = 0? This seems unneeded
|
||
@SpireInsertPatch(locator = Locator.class) | ||
public static void Insert(AbstractMonster __instance) { | ||
if (AmbushedUtil.isInAmbushedZone() && AbstractDungeon.player.hasPower("Surrounded")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use string literals for ids. Use SurroundedPower.ID etc
|
||
@Override | ||
public void replaceRooms(Random rng) { | ||
replaceRoomsRandomly(rng, MonsterRoom::new, (room)->room instanceof EventRoom, 100); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just want to replace all event rooms?
@Override
public void replaceRooms(Random rng) {
//Replace all event rooms with monster rooms
for (MapRoomNode node : this.nodes) {
if(node.room != null && EventRoom.class.equals(node.room.getClass())) {
node.setRoom(new MonsterRoom());
}
}
}
This is more effective
|
||
private void initCardDrawCardIDs() { | ||
cardDrawCardIDs = new ArrayList<>(); | ||
cardDrawCardIDs.add("Pommel Strike"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the ID field every card has.
Also, if you want this to include modded characters you could ask ali for the code to dynamically check if a card is a draw card.
src/main/java/spireMapOverhaul/zones/ambushed/AmbushedZone.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
@Override | ||
public void postCreateShopPotions(ShopScreen screen, ArrayList<StorePotion> potions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not a good idea. Maybe just replace one potion with a draw potion. This seems overkill, and yoU're unlikely to want this many anyway. Also too much repetition in the potion choices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make the argument that due to the amount of zones there are and how small my zone is in terms of height + width, the odds of someone naturally wandering into a shop that happens to fall in my zone are very slim.
Also, replacing just one potion with a draw potion would be too petty to add as a modification I think. I am satisfied with the current setup, although maybe I could make it so one potion of each rarity is guaranteed to appear? So the player has the option to purchase a Swift Potion, a Gambler's Brew, and a Smoke Bomb (lol) / Snecko Oil. I do think three Swift Potions in one shop feels pretty bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'd be okay with that.
} | ||
|
||
@Override | ||
public void modifyReward(RewardItem rewardItem) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, don't think this is a good idea since it's always the same potions.
new ZoneEncounter(AMBUSHED_BY_SENTRIES, 1, () -> new MonsterGroup( | ||
new AbstractMonster[] { | ||
new Sentry(-1000.0F, 0), | ||
new Lagavulin(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use the combatmodifying zone to check for the laga encounters and nerf it a bit? This does seem a lot harder than the base game elites
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the idea of having inconsistency in the game, where loading into a specific encounter nerfs a specific enemy. I'd rather try to find another enemy to replace Sentry with. I don't think the Sentry itself is that bad of an addition, it's the Dazes it adds that will REALLY mess a player up, especially if Laga debuffs them and their next turn is just full of Dazes.
Co-authored-by: erasels <[email protected]>
Co-authored-by: erasels <[email protected]>
So for the reward modification, here's how I see it. The player is really looking for those high roll card draw cards that will help them throughout the rest of the game. We're talking Master of Strategy, Scrawl, Reboot, Acrobatics even. Therefore, I believe having 3 options per fight vastly improves how "good" the zone feels, as the odds of getting a high roll draw card are fairly reasonable. If my zone only had 1 card draw choice per fight, you could see something like Prepared, then Impatience, then Escape Plan, then oops, you're out of the zone and you took all those hard combats for basically nothing. I think the current configuration is reasonable. As the zone only appears in acts 1 and 2, the zone gives you the chance to pick cards that will help your deck throughout the rest of the run, as card draw is always nice to have. Also to answer all of your "why did you code it like this" questions, I do not know Java myself so I had ChatGPT program this whole thing. I will do my best to address some of your concerns and then update the PR. |
I see your point in regards to trying to get the highroll draw cards, however I believe that there's an issue with it. The thing you may be discounting here is that drawing cards is only good if you have things worth drawing. This zone can spawn in both act 1 and 2 where players are likely to be still building their deck and pick up value cards that help them win. You would not want to pick up an acrobatics when you have few high value cards or syngeristic effects. Also, your code quality is fine, that locator just struck me as very weird. The others are just small fixes, I'm sure we'll get there. |
} | ||
|
||
@Override | ||
public List<ZoneEncounter> getEliteEncounters() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The normal fights make sense to me since they're all ambush variants of existing normal fights. The surrounded mechanic means that you'll take somewhere around 25% more damage if you can't kill one enemy quickly, with the chance for more if you aren't able to turn around on a key turn. That can be deadly, but the zone is advertised as highly dangerous, so sure. I do think there's a risk that some of these fights can spike damage high enough in the first few turns that it's just not fun for the player, but that's something we can work out in playtesting (if it turns out to be an issue, we have options like giving the monsters weak on turn 1).
The elite encounters are where this might need some adjustment. I've made some fights like this (existing elites plus an extra enemy) as part of Corrupt the Spire, so while those didn't use the ambush mechanic, I have some idea of how difficult these will be.
For the act 1 fights, Louse and Nob seems close to the right balance. Having the Louse is like a mix of the burning elite strength and HP buffs (it's a bit more HP to work through and a bit more potential damage). This is more of an increase in difficulty relative to the original fight since it adds a new enemy (compared to the normal fights which don't add new enemies), but it's not that much harder.
Sentry and Lagavulin, on the other hand, is a huge amount of HP to get through. Even with Lagavulin sleeping at the start, if you can't kill the Sentry very quickly, the dazes make the long-term endurance fight against Lagavulin even harder. This seems like too much, even for what this zone is aiming for. There's a couple ways you could go instead. You could make a hard fight just by having Lagavulin be already awake and adding an small Acid Slime, or you could have a sleeping Lagavulin together with something that's less dangerous than a Sentry (a medium Acid or Spike Slime, perhaps).
For the act 2 fights, I get that they're the two Colosseum fights, but they don't match the difficulty I'd expect. Based on the act 1 elite fights for this zone, the idea seems to be that they're tuned to be more dangerous than normal elite fights in act 1 (since they add an extra enemy and surrounded). In contrast, the two slavers fight is less dangerous (I'd rather face the two slavers ambush than the normal three slavers elite fight). And while Taskmaster and Nob is harder to evaluate, it still feels like its in a weird place. Also, the two slavers fight doesn't have any enemies that count as elites, which is a bit too weird.
Replacing the two slavers fight with Book of Stabbing plus a weak enemy would give you one encounter that has around the right balance. I'm not sure what to suggest for the Taskmaster and Nob fight, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated elite encounters make sense to me.
Having thought about it more, keeping Taskmaster and Nob seems fine. It can be a dangerous fight (especially if you aren't able to kill Nob quickly), but being dangerous is in line with the other elite encounters in this zone, and I don't think it's out of line with the other elite ambushes you've made. So let's give it a shot, and if it turns out to be too much, we can replace or change it.
One final thought on these encounters. Given that they're harder than normal, it may be worth considering at least a small additional reward. I get that if you're looking for card draw, getting a card reward with multiple card draw options is already a boost. But it might be worth something more on top of that, such as:
- A small amount of extra gold
- Having card rewards be more likely to be upgraded
- An extra guaranteed potion
I don't think it's critical to the zone to have this, but right now my intuition is that the risk/reward of this zone may be slightly too skewed towards risk. Can't be sure of that without playing with it a bunch, but together with Gk's feedback, it supports the idea that a bit more reward may help balance things out. (Depending on what you go with, having normal encounters also get a small extra reward might make sense.)
case RARE: | ||
// Randomly select between Smoke Bomb and Snecko Oil for Rare potions | ||
if (AbstractDungeon.cardRandomRng.randomBoolean()) { | ||
replacementPotion = PotionHelper.getPotion(SmokeBomb.POTION_ID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting smoke bombs feels like a troll. Taking harder fights and having part of the "reward" be a near-useless potion isn't a great experience.
I'm not sure about the whole potion replacement thing either, but if you're doing it, it would land better if you cut the smoke bomb thing and just gave out Snecko Oils.
# Conflicts: # src/main/java/spireMapOverhaul/zones/ambushed/AmbushedZone.java
I made the following changes:
|
First time ever doing a PR. Hopefully I didn't botch anything.
Known points of concern for the zone: