[PATCH] Moddable cheats

[Matthijs de Rijk]

simulations/data/cheat.json now is not moddable in a way that it will override the original file. A suggestion for this is putting cheats in a folder instead of in one file. In case of readability maybe it's better to write the cheats in xml instead of json.

when handling duplicates when combining, the cheat script just should take the first occurrence of the cheat with that name.

[O.Davoodi]

I'm against putting the cheats in an XML. JSON isn't unreadable and the cheat system is purely in JS, so it'd be better we don't change the file system.

[lsdh]

I made some changes that would satisfy the first demand. The change from json to xml is not done (see previous comment).

The patch add a new folder with all the cheats in separate .json files and the necessary code (a function) to take into account all the .json file of the folder except it the "Name" attribute is already used.

All the new .json files are unversionned. The old cheats.json file is useless once the patch applied (so should be deleted).

(All cheats works except TARDIS, but this is not related to this patch, see ​http://trac.wildfiregames.com/ticket/2045).

Patch updated thanks to some Spahbod's recommendation on variables' and function's name. Didn't succeed to put the function in Cheat.js file (I have to check when this file is loaded, the messages.js seams to be loaded just after the map's loading) .

Do not hesitate to criticize my work, I'm quite a newbie.

[leper]

Check SubmittingPatches when submitting patches (especially the part about the review keyword).

Some comments:

• Change the cheat json data to something like

  {
    "Name": "wololo",
    "Data": {
      "Action": "convertunit",
      "IsNumeric": false
    }
  }
  

  Also do we really need the IsNumeric in each json file? Or does that throw a warning when accessing it, it shouldn't but I haven't checked (access as in if(mycheat.IsNumeric) ...)
• Rename cheatList to cheats (or something like that)
• getCheatsData
  • Change the for to a for each (var filename in filelist) and make cheats an object instead of an array (will come in handy later)
  • That way you can make the check if a cheat is already present (the name that is) to be something like

    if (cheats.indexOf(currentCheat.Name) !== -1)
      warn("Cheat '"+currentCheat.Name+"' is already present");
    else
      cheats[currentCheat.Name] = currentCheat.Data;
    

• Change the current cheat detection in messages.js (lines 222-252 without the patch) to iterate over Object.keys(cheats) instead. Also that whole check should only be done if cheats are enabled, but that isn't really a problem if it isn't fixed as a part of this ticket.

[lsdh]

Thanks leper for this really good feedback.

I'll make the changes tomorrow (going to sleep right now).

[lsdh]

Replying to leper:

Also do we really need the IsNumeric in each json file? Or does that throw a warning when accessing it, it shouldn't but I haven't checked (access as in if(mycheat.IsNumeric) ...)

Each time there is a "IsNumeric", there is also a "DefaultNumber".

The test "IsNumeric" is used to capture the number that the player will enter after the cheat. I think an easy way to suppress the "IsNumeric" would be to check if the property "DefaultNumber" exist.

So, instead of

if(cheat.IsNumeric)

, we could have

if(typeof(cheat.DefaultNumber)!='undefined')

[leper]

Make that if (cheat.DefaultNumber !== undefined) and we're good to go.

[lsdh]

Replying to leper:

• Change the current cheat detection in messages.js (lines 222-252 without the patch) to iterate over Object.keys(cheats) instead.

• Also that whole check should only be done if cheats are enabled, but that isn't really a problem if it isn't fixed as a part of this ticket.

• I have it working with:

for each (var cheat in Object.keys(cheats))

However cheat is only a string with the name, not the object. It ease to test if the text begin with this name:

if (text.indexOf(cheat) == 0)

but when I need the properties of the object, I have to call them with

cheats[cheat].DefaultNumber

to have the DefaultNumber properties.

Is it right? Would'n it be better to have a:

for (var cheat in cheats)

And directly use the object cheat?

• By now, the check seems to be done in Cheat.js:

  if (!cmpPlayer.GetCheatEnabled())
  	{
  		cmpGuiInterface.PushNotification({"type": "chat", "player": input.player, "message": "Cheats are disbaled in this match"});
  		return;
  	}
  

(apart of the two previous points that can/could be enhanced, I done all the other recommendations from leper. I will add the new patch once this two last points will be done (and once I tested if all cheats still works as before).

[leper]

Replying to lsdh:

• I have it working with:

[...] Is it right? (it works, but it may be better in an other way).

Yes, it is.

• The option to enable/disable cheats is accessible when we play in multiplayer . I suppose this is a boolean variable that I could check at the beginning of the two functions (no need to load and no need to test if in the list if cheats are not allowed). I go searching for this variable.

You'd have to add "cheatsEnabled": cmpPlayer.CheatEnabled(), to GuiInterface::GetSimulationState() (in simulation/components/). Also maybe change that function name and all call-sites to CheatsEnabled() (but that isn't really important, but if you're at it already ;-) ). Another thing would be to change the default in cmpPlayer::Init() (Player.js in simulation/components/) to cheatsEnabled = false;. In simulation/helpers/InitGame.js remove the cmpPlayer.SetCheatEnabled(true); for AIs (line 24) and also make the other usage of cmpPlayer.SetCheatEnabled(false) (within the if) to be cmpPlayer.SetCheatEnabled(!!settings.CheatsEnabled); (without if) (double ! to pass a boolean).

(Now we have cleaned up the cheat init on the simulation side a bit, and are passing the cheat state to the gui (that is what the GuiInterface modification is for)) Now we need to add "cheatsEnabled": playerState.cheatsEnabled, to getPlayerData in utility_functions.js and then check if cheats are enabled by using if (g_Players[Engine.GetPlayerID()].cheatsEnabled) ...

(apart of the two previous points that can/could be enhanced, I done all the other recommendations from leper. I will add the new patch once this two last points will be done (and once I tested if all cheats still works as before).

(just noticed your edit: That check is done after passing the cheat (which could be a valid, though probably not really useful chat message otherwise) to the simulation (the part of the code were the game happens, which doesn't share data with the gui (apart from GuiInterfaceCalls and PostNetworkCommands) so we have to add some code to pass data between those two contexts (see GuiInterface))

(hopefully that mess I posted is readable, if not just ask)

[leper]

Also while you're changing messages.js replace the two calls to getPlayerData() in there (and the additional variable) with g_Players.

[lsdh]

Once getPlayerData() replaced by g_Players does

var playerData = g_Players;

So I replace all playerData with g_player to clean a bit the code.

[lsdh]

Patch proposal

[leper]

In 13620:

Split up cheats into multiple files. Patch by lsdh/ldsh. Fixes #2019.
Replace buggy hasAllies with the inlined check if we have a team at all.
Fix setting of starting resources (broken in r13044).

[leper]

Thanks for the patch. (You're known as lsdh here and as ldsh on irc, so I included both nicks in the commit message)

I made the loop that checks if a cheat is submitted to continue on mismatches (less indentation and makes the diff a bit more readable) I also made the /command handler code use less indetation (switch style as per the Coding Conventions) (unrelated to your patch) and inlined the hasAllies code (which was broken anyways) after some discussion with alpha123 on irc. (Your patch didn't replace the playerData in the first hasAllies() btw)

The tests were broken so I fixed them too (run the test binary right next to the pyrogenesis binary to check if you didn't do that).