-
Notifications
You must be signed in to change notification settings - Fork 44
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
Feature flags in the game and scenarios #584
Conversation
Recommend editing |
It seems like something is missing here. I don't see any code that initializes the feature flags when starting a new scenario. |
The scenario's feature flags are added to the scenario by the editor when it creates it. |
But the game only uses the global feature flags. The scenario feature flags have no effect. Plus, setting the feature flags to empty in a new scenario doesn't seem correct. Or to put it another way, it looks like there is nothing to make the feature flags actually do the thing they're supposed to do, namely making the shirt block 1 damage if you play a legacy scenario and 0 damage if you play a newly-created scenario. |
Oh, I wasn't concerned with keeping shirt defense at 1 for legacy scenarios. That just seems like a bug. The feature flag is there for the hypothetical scenario where a replay might exist where the bug is pivotal. (In this case, I don't think one does.) |
I care more about the feature flag infrastructure than the shirt defense thing. |
It does seem like a bug, but the shirt defense thing also doesn't seem like a good example of when you'd want a feature flag. Isn't the feature flag system supposed to be a generalization of the scenario "legacy" flag? With this implementation it definitely doesn't seem to be doing its job. Maybe we have different ideas of what it's supposed to do though. But, even considering that, it doesn't make any sense to add scenario feature flags that do nothing at all. |
The scenario editor and the game will both be able to check for scenario feature flags when we do need to add any. |
I'm going to separate out the armor fix as its own PR. |
fd0d339
to
cb67104
Compare
Retooling this for a different PR. |
I've implemented my idea for the feature flags system. The game has feature flags which are recorded and replayed so we can keep legacy behavior the same in replays. As an example, I've fixed the shirt bug mentioned in #245 in a way that should keep old replay behavior the same (not that we have any replays that had the shirt bug in them...)
I also gave scenarios feature flags which we'll eventually be able to use to detect in the editor whether anything needs to be migrated to a new format to leverage a new feature.
Fix #555