PLAYER_DEFAULT_SCIENCE_RATE=100 leaks into game start in govs not allowing it
lexxie9952 lexxie9952 So the scenario had players that had both "Despotism" as their government, and science rate of 100? Then it would be a bug in that scenario file.
Reply To cazfi
So the scenario had players that had both "Despotism" as their government, and science rate of 100? Then it would be a bug in that scenario file.
Update: That's not where the bug is. I have received more information from players. The bug happens in all games, including games using a random generator and no scenario. This bug does not happen in singleplayer games, including games using that scenario.
I believe this is a specific problem to how Freeciv handles human players joining into multiplayer games. Specifically, unassigned player slots taken by joining players. In many cases the joining player does get the 60% despotism science rate forced onto them. But in some cases, not. Whatever code is meant to catch and override the 100 value that is put into the pplayer struct when it's created, this code does not catch every case. The software allows ways for players to take over one of these pplayer structs and circumvent those checks and start playing with a 100 as their science rate. This is probably because the messy dirty world of multiplayer games has events like people joining late, taking over unassigned slots, status of the pplayer toggling between unassigned/AI/human, admins running commands to change an AI to a human, and so on.
The possibility for this bug was decided long ago when someone decided to make all new *pplayer structs get allocated with an illegal PLAYER_DEFAULT_SCIENCE_RATE=100. That rate is not allowed in almost any known ruleset. For a bug not to happen they would have to be super-genius to imagine and catch every possible case of the pplayer status getting changed, and monitor it on every turn change and every place where tax rate income is ever calculated. They failed. They thought "no problem, code will later catch this and change it to the legal rate." They were wrong. There are holes in the system, and the consequences of setting an illegal rate in the initial allocation are now coming home and knocking on our door.
Just so you know... this has no urgency for us at FCW anymore. FCW just changed this default rate to 60 in the player.h file. Voila! The problem is gone (for us.) I just wanted others who are still using the 100 rate in players.h to be aware of two things: 1. This type of coding can create the bug I described. 2. Changing the .h file doesn't truly fix the bug that is not catching the illegal rate, and it could affect other rulesets (e.g., some hypothetical ruleset where max rates in starting gov are at 40%.)
Unlike you imply with "catch every possible case", there's just two cases: initialization of a new player, and aitoggling from an AI player allowed to cheat. Both of those are supposed to limit rates to current max.
The problem is likely in get_player_maxrate() returning 100 when it cannot determine correct value to return. In a typical ruleset where rates are property of the government, that might happen when attempt to clip rates is done before player's government is initialized. Likely unrelated, player_limit_to_max_rates() seems to have also a bug that it allows *any* AI to cheat.
Alien ruleset has starting rates at 40% - not a "hypothetical" ruleset but one that is part of the distribution.
In any case my initial idea that the rates loaded from a scenario might be accepted as they are turned out to be true. Split that to a separate ticket -> #44186, as you claim that this one is something else.
Reply To cazfi
The problem is likely in get_player_maxrate() returning 100 when it cannot determine correct value to return. In a typical ruleset where rates are property of the government, that might happen when attempt to clip rates is done before player's government is initialized.
Going through all the cases, I found any place where that would be an issue in freeciv.org code. Nor have I managed to reproduce any such problems with any test setups (except the separate savegame loading bug)
However, looking at FCW code there's our old friend, attach_longturn_player(). That at least has one obvious bug resulting in illegal tax rates, as it just flags former (possibly cheating) AI player to a human one. Well, there's more that it fails to do. Likely just replacing the set_as_human() call with player_set_under_human_control() works.
Reply To cazfi
Reply To cazfi
The problem is likely in get_player_maxrate() returning 100 when it cannot determine correct value to return. In a typical ruleset where rates are property of the government, that might happen when attempt to clip rates is done before player's government is initialized.
Going through all the cases, I found any place where that would be an issue in freeciv.org code. Nor have I managed to reproduce any such problems with any test setups (except the separate savegame loading bug) However, looking at FCW code there's our old friend, attach_longturn_player(). That at least has one obvious bug resulting in illegal tax rates, as it just flags former (possibly cheating) AI player to a human one. Well, there's more that it fails to do. Likely just replacing the set_as_human() call with player_set_under_human_control() works.
Thanks! What else does it fail to do, if I may ask?
Reply To (Anonymous)
Reply To cazfi
Likely just replacing the set_as_human() call with player_set_under_human_control() works.
Thanks! What else does it fail to do, if I may ask?
Everything that player_set_under_human_control() does, except set_as_human()
Update...
Now I'm really confused.
attach_longturn_player() has this hard-coded right into the end of the function:
So, yeah, I'm confused. I can only guess that get_player_maxrate() returns 100 at some other point later, and something about the multiplayer context of people joining at different stages in the game (before it starts, when it starts, after it starts, and being assigned by an admin to be in the game before they ever really connected to the game) -- that somehow in one or some of these cases, something is falling between the cracks.
It might have to do with the admin setting up the unassigned players with the right username before they ever attach, so it circumvents the usual checks?
I will do the player_set_under_human_control() as you suggest, but now I'm sceptical that this fixes the bug since it's hard-coded to change it to 60/40. Also that function won't do it if the flag for it being a new game is on (but once again shouldn't matter because it's hard-coded to 60/40 later.)
if it helps deduce the problem, when testing this bug and going into the tax sliders, it showed as gold 0 lux 0 sci 60. Using only 60% of the available 100. I say this only if it sparks some idea where might be causing it.
Reply To (Anonymous)
attach_longturn_player() has this hard-coded right into the end of the function: // default tax rates pplayer->economic.science = 60; pplayer->economic.tax = 40;
This happened in a Team-Game where the GM/admin did a custom setup because of filling the game with players registering for it into teams.
The GM selected a scenario map with x number of players in their starting positions, as unassigned players. As the sign-ups for the teams were filled, the GM manually converted the unassigned players into usernames of humans, so that they'd take over that nation on first login. Then the players, in Despotism, inherited a science rate of 100% -- which was never checked nor caught by the server. They were able to keep this rate for multiple turns until the bug was discovered.
At FCW, for now we just did a solution but not true bug fix. We changed PLAYER_DEFAULT_SCIENCE_RATE to 60 and PLAYER_DEFAULT_TAX_RATE to 40 (which it should probably be regardless.)
However, it's conceivable in some rulesets that those rates would still be illegal in a starting government and thus this bug would still remain.