3.7.1: Lack of error handling in case of modified game data

[tomreyn]

While donig so is a very bad idea for several reasons and I keep telling people so, it's a fact that there are several people who, instead of using separate techtrees with faction links, add extra factions to Megapack. This can result in (non-fatal) errors like the following:

Code: [Select]

[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 0 items[idx] = [Egypt]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 1 items[idx] = [Indian]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 2 items[idx] = [Magic]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 3 items[idx] = [Norsemen]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 4 items[idx] = [Persian]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 5 items[idx] = [Romans]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 6 items[idx] = [Tech]
[2013-03-06 17:26:29] *ERROR* In [components.cpp::setSelectedItem Line: 433]
instanceName [listBoxFactions4] idx = 7 items[idx] = [*Random*]
[2013-03-06 17:26:30] *ERROR* In [menu_state_custom_game.cpp::switchSetupForSlots Line: 2166] Error [[listBoxFactions4] Value not found on list box: [Japanese]
Stack Trace:
./megaglest:Shared::Platform::megaglest_runtime_error::megaglest_runtime_error(std::string const&, bool)address [0xb18eae] line: 237
./megaglest:Glest::Game::GraphicListBox::setSelectedItem(std::string, bool)address [0x619169] line: 435
./megaglest:Glest::Game::MenuStateCustomGame::switchSetupForSlots(Glest::Game::SwitchSetupRequest**, Glest::Game::ServerInterface*&, int, int, bool)address [0x86374e] line: 2146
./megaglest:Glest::Game::MenuStateCustomGame::update()address [0x88024c] line: 2313
./megaglest:Glest::Game::Program::loopWorker()address [0x776493] line: 434
./megaglest:Glest::Game::glestMain(int, char**)address [0x768ff6] line: 4793
./megaglest:Glest::Game::glestMainWrapper(int, char**)address [0x76c703] line: 4973
/lib/x86_64-linux-gnu/libc.so.6:__libc_start_main()address [0x7f9331d9576d] line: 0
./megaglest() [0x5db3e9]address [0x5db3e9]
]
From my point of view, people  with broken game data who join a server I host should be unable to trigger errors on my side. To this end, it may be necessary to handle this situation better, which could include disconnecting players with such modified game data.

[-Archmage-]

Well for things like added factions, it should detect that in the lobby and disable use of any extra files in the game. For modified files, I think during the loading process it should, and handle these issues as logically as possible. Image/Texture/Model/Sound modifications would be ignored, for the techtree and tileset alike, and any XML modifications detected it would pause loading and ask the player if they would like to patch the files from server/player.

Kicking people out of the game for having an extra faction in their pack is not good multiplayer practice, you have no idea how confusing everything is for newbies, the game should handle modding as friendly as it can. This enables the best experience. Plus the solution I have written would enable the gamedata(except for XMLs) to be patched without throwing out people that don't have the patch.

I'm sure it's quite a bit of work to design a more complicated solution like this, but this is a solution that would better support modding, and do as much as possible to get that player in the game instead of facing an error message.

[tomreyn]

To clarify better, I am discussing manual modifications to the game data we ship, i.e. modifications to game data which is not meant to be modified, ever, in the first place. This is data stored in the installation location, usually next to the binary, not the data stored in a location where users are allowed (both by the OS / file system permissions / hierarchy standards and by means of how MegaGlest is designed) to make modifications to game data (next to glestuser*.ini, downloads from the mod menu and from other servers).

What you're suggesting as a solution implies that you'd define a game server to always be the authoritative source for any game data, including Megapack. So any player could host a modified copy of Megapack, someone would connect to this server, have it downloaded to their system and this wrong copy of the Megapack would spread this way. That's not what we want, plus it seems wrong in terms of design to allow for modifications to shipped game data this way in the first place.

[-Archmage-]

Well I don't like using the data folder, and most people who play glest will never know of it's existance.
Ok, so don't compare to the host, somewhere, maybe the master server or a protected and compressed archive could store the XML data and it would simply compare gamedata in the background in the lobby and on the loading screen. Once it has established which files need to be patched it could simply patch them or you could have the game repair itself. It knows all the modded xmls, so it copies them over to the user folder so that they can be used for singleplayer, and patches the original gamedata in the main folder. On top of that, you'd want a quick way to detect whether it is necessary to compare xml files in the first place.

Something like this would be best, something that fixes everything and gets the player in-game without overwriting their work or throwing them out for cheating with a generic error message. Even after you tell the player that their data is different how are they to fix that? Download all the gamedata and overwrite? That takes a while, they no longer can play in that game it will be over before they are done.

[tomreyn]

Assume you acquire a Flying Pigeon style bike, with frame, drivetrain, gearing, steering, seating, wheels tires and pedals, which works fine pretty much on every road, and all kinds of other traffic on those roads. Then you replace its front wheel by a hat sized stone, because you feel like it. Would you then expect it to work on those streets and in general traffic there? Would you expect it to magically repair itself?

The MegaGlest team ships a software, which  (with the possible exception of unintended bugs) works as advertised and is compatible to all other copies and variants, even across different platforms. Now if someone willfully replaces part of this software by something else, in a way it is not meant to be done and not supported, should this someone still be able to expect it to work or magically fix itself? Especially when this software does provide well usable means to extend its abilities in a standard and supported way?

To me, the obvious answer to this question is: No. And I can't see how anyone could have different expectations.

As a side note, this is a bug report about MegaGlest stability when it comes across unexpected game data. It is not a request for discussion of how MG should deal with incompatible game data nor a feature request. Your opinion is appreciated, but this ('bug reports') is the wrong place for having this discussion (feel free to start it elsewhere).

[-Archmage-]

Luckily for us MegaGlest is not a Pigeon bike, it's a piece of software, and copying files around is a very simple process for a computer compared to just about everything else the engine has to do. I don't expect software to be amazing, but I think my idea would be a much nicer way for handling newbs, modders, and everyone else. You guys are the ones that decided to make modding more complicated anyway.

[tomreyn]

This topic is not about changing the games' design. Please start a new post in the feature requests forum if you would like to suggest to change the design.

This topic is about how there is a deficiency in the game server handling the situation where a clients' game data has been tempered with in a non-supported way. The details are in the first post, so I won't reproduce those here.

I don't know whether this still occurs in r4246 (3.8-dev), but I would assume it probably does. It would be good to verify this in a multi-player test.

Thanks.