Skip to content
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

Add Keys#PREVIOUS_GAME_MODE #2442

Merged
merged 3 commits into from
Oct 31, 2022

Conversation

Lignium
Copy link
Contributor

@Lignium Lignium commented Sep 2, 2022

[SpongeAPI | Sponge]

@Zidane
Copy link
Member

Zidane commented Sep 2, 2022

@Lignium

I wonder if we should allow this key to be set? I only bring this up because setting it effectively feels like breaking the contract of what it is?

@Lignium
Copy link
Contributor Author

Lignium commented Sep 2, 2022

@Zidane My authorization plugin manipulates game mode, adventure is set for the duration of authorization. And there was a problem, the key combination F3 + N broke, the previous game mode turned out to be the mode that temporarily set the authorization plugin, not at all what was expected. Instead of switching back and forth between creative and spectator, I get thrown into adventure mode and falling from a height.

@Zidane
Copy link
Member

Zidane commented Sep 2, 2022

My point is I'd expect setting of GAME_MODE via key would change PREVIOUS_GAME_MODE but no one can rely on PREVIOUS_GAME_MODE actually being truthful if anyone can simply go set it via API contract.

@Lignium
Copy link
Contributor Author

Lignium commented Sep 2, 2022

One option is to make it read-only, and with two Keys#GAME_MODE set in a row, I can set both values.

@gabizou
Copy link
Member

gabizou commented Oct 5, 2022

The way I see it is that while game modes can change, we easily provide the DataTransaction including the Keys.PREVIOUS_GAME_MODE, since it can very well be changing.

@Faithcaio Faithcaio merged commit 0ed879e into SpongePowered:api-8 Oct 31, 2022
@Lignium Lignium deleted the feature/previous-game-mode branch November 5, 2022 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants