-
Notifications
You must be signed in to change notification settings - Fork 77
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 all classic S2 cheats and a few more #1679
base: master
Are you sure you want to change the base?
Conversation
…ass "CheatCommandTracker"
Wow, great effort. |
|
||
bool Cheats::canCheatModeBeOn() const | ||
{ | ||
return world_.IsSinglePlayer(); |
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 we can extend this to be a setting inside the create game screen, to allow cheats in multiplayer too
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 could certainly create an "enable cheats in multiplayer" addon or something similar, but it would be useful to ask which of these cheats make sense in multiplayer. I don't really know a reason to enable most of them since they are so game-breaking.
On the other hand, I guess they could help in debugging, giving someone a handicap (experienced player sees FOW, newbie sees everything; experienced needs geologists, newbie sees resources, etc.) or eliminating the need to send geologists anywhere.
std::unique_ptr<CheatCommandTracker> cheatCmdTracker_; | ||
bool isCheatModeOn_ = false; | ||
bool isAllVisible_ = false; | ||
GameWorldBase& world_; |
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.
we should use a std::reference_wrapper
instead of a plain reference
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 quite see what the benefit of reference_wrapper
would be here? world_
is literally meant as a run of the mill C++ reference. Also there is just one use of reference_wrapper
anywhere in RTTR code (excluding turtle) and it's in s25util where it is inside a std::optional
, where a plain reference cannot be.
@@ -59,6 +60,7 @@ class GameWorldBase : public World | |||
EventManager& em; | |||
std::unique_ptr<SoundManager> soundManager; | |||
LuaInterfaceGame* lua; | |||
std::unique_ptr<Cheats> cheats; |
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.
don't know if this needs to be an unique_ptr? can't you simply call the ctor in GameWorldBase ctor?
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.
No reason other than to forward-declare instead of including. This can just be Cheats cheats;
if this is preferred. I didn't want to recompile so much code when working on the Cheats header but I can see the benefits of removing the unique_ptr crap (clarity, unnecessary to include Cheats.h in some other places).
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'll leave the unique_ptr. Including Cheats.h from GameWorldBase.h is just a pain and several other members of GameWorldBase are also unique_ptr's (presumably for a similar reason). Please unresolve if you disagree.
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.
Ah yes, it's another cyclic dependency here. Moving it out of the world should resolve it.
I don't think we should enable cheats in multiplayer, even with addons, I don't see a use case. For balancing reasons we should rather add some real balancing options instead of cheats (which most people are not aware of I think) |
I will place a PR for the first three commits of this one and then add one PR per cheat/feature. I didn't want to do this in the first place because the refactoring may have seemed pointless, but now that a couple of people have seen the results this can bring, I think the first few commits can be merged without much issue and then each cheat can be considered on a case-by-case basis instead of a big blog that is difficult to review. |
I agree. It should only be for debugging/map debugging or some fun, not disturb multi-player games |
I would happily split these cheats into separate PR's, but they all rely on the refactoring, so at least the first few commits would have to be merged before any split can be done.
The all-visible and build HQ cheats are useful for testing campaign scripts, which I have also been working on. The remaining S2 cheats are there mostly for feature parity and setting speed does not work the same as it did in S2 (certainly not fast enough, probably steps are different too).
The reveal resources cheat can be useful not only in gameplay but in debugging as well and the destroy buildings cheat is useful for testing the world campaign progress where the player needs to achieve total domination but also not destroy themselves (as they would with the armageddon cheat).
Related to issue #72