Page MenuHomeWildfire Games

Allow setting modifiers while upgrading.
Needs ReviewPublic

Authored by wraitii on Jan 19 2021, 2:07 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

As noted regularly, upgrading is powerful because it requires no workers.
This diff allows setting modifiers on the upgrading entity while it is being upgraded, which can make the choice to upgrade more tactical.

It introduces upgrade for Cart houses to Appartment, by temporarily lowering population (NB: there's no checking here, so the pop limit can be 'cheated' a bit).

Could/Should also be used for tower upgrades (do we have those?) and so on IMO.

TODO:

  • Would perhaps be nice to set this as a status effect so we can have an icon? 'Tis a bigger diff.
Test Plan

Agree that this is a nice feature.

Event Timeline

wraitii created this revision.Jan 19 2021, 2:07 PM
Owners added a subscriber: Restricted Owners Package.Jan 19 2021, 2:07 PM

Build is green

builderr-debug-macos.txt
ld: warning: text-based stub file /System/Library/Frameworks//CoreAudio.framework/CoreAudio.tbd and library file /System/Library/Frameworks//CoreAudio.framework/CoreAudio are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox.tbd and library file /System/Library/Frameworks//AudioToolbox.framework/AudioToolbox are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback.tbd and library file /System/Library/Frameworks//ForceFeedback.framework/ForceFeedback are out of sync. Falling back to library file for linking.
ld: warning: text-based stub file /System/Library/Frameworks//CoreVideo.framework/CoreVideo.tbd and library file /System/Library/Frameworks//CoreVideo.framework/CoreVideo are out of sync. Falling back to library file for linking.
ld: warning: text-based stu

See https://jenkins.wildfiregames.com/job/macos-differential/2967/display/redirect for more details.

wraitii requested review of this revision.Jan 19 2021, 2:20 PM
Stan added a subscriber: Stan.Jan 19 2021, 2:21 PM

What kind of icons would you need?

In D3424#151616, @Stan wrote:

What kind of icons would you need?

It's more of a JS concern, modifiers don't appear in the GUI, whereas status effects do.

Stan added a comment.Jan 19 2021, 2:27 PM

Kind of feel like the duplication should be reduced no?

In D3424#151620, @Stan wrote:

Kind of feel like the duplication should be reduced no?

There's no duplication?

bb added a subscriber: bb.EditedSep 9 2021, 4:57 PM

Sounds like a great addition. Now I am not so sure if doing this on the cart houses is good gameplay wise (it renders the up pretty useless if one first loses pop to get a little more, s one won;t need to have more houses to start). I would more see a usecase in towers losing there arrows during upgrade.

Would be nice if we have some identification in the gui

code looks good, didn't test

binaries/data/mods/public/simulation/components/Upgrade.js
265

const

266

if two upgrades are done simultaneously we give them the same name here, maybe we should add the this.upgradeTemplates[template] to the name too?

289

const

In D3424#181342, @bb wrote:

I would more see a usecase in towers losing there arrows during upgrade.

That perhaps sounds more like #5889? (Upgrading state so one cannot be in the attacking state.)

In D3424#181342, @bb wrote:

Now I am not so sure if doing this on the cart houses is good gameplay wise (it renders the up pretty useless if one first loses pop to get a little more, s one won;t need to have more houses to start).

For what it's worth, this was intended to make upgrade less good, as it's currently very efficient since it takes no builder (something that e.g. Nescio didn't like in the Tower case IIRC). Giving a temporary debuf allows balancing the cost in resources & build time vs the ultimate bonus. Whether the current balance is good is quite debatable.

Would be nice if we have some identification in the gui

IIRC this is actually why I didn't finish / merge this > I felt as if I should re-use status effects for it, and then ran in the issue that I wanted to generalise the 'show an icon & description in the GUI for a unit' component, but ended up having some trouble finding a clever way to do that.
The problem is that the translation needs to be, on some level, simulation side.

binaries/data/mods/public/simulation/components/Upgrade.js
266

Is that possible? Still, probably not a bad call.