Follow-up Comment #2, bug #14230 (project freeciv): > - This change may make sense, but I mention it anyway: plrhand.c:182-> you > replace "S_S_RUNNING != server_state()" with "!server_state_started()". > This changes behavior as latter accepts S_S_OVER.
which is valid as it is wrong if one tries to change the rates in S_S_OVER > - plrhand.c:678-> you replace "assert(S_S_RUNNING > server_state() ..." > with "assert(server_state_started() ..". Former accepts S_S_INITIAL & > S_S_GENERATING_WAITING, latter S_S_RUNNING and S_S_OVER you are right; the calls in line 681 and 688 have to be switched > - plrhand.c:1111-> you replace "S_S_INITIAL != server_state()" with > "server_state_started()". Former accepts S_S_GENERATING_WAITING, latter > does not I thing its line 1522. S_S_GENERATING_WAITING is used as transient state between initial/savegame and running. It is only entered after _all_ player are ready. But it could be that some error resets the game while generating - changed to '!server_state_pregame()' > - I think this one is a bugfix: savegame.c:4588-> "S_S_RUNNING == > tmp_server_state" replaced with "tmp_server_state > > S_S_GENERATING_WAITING". Latter will catch also S_S_OVER > - sernet.c:571-> you replace "S_S_INITIAL != server_state()" with > "server_state_started()". I think this one is harmless as this code path is > never executed during S_S_GENERATING_WAITING anyway. ... Rethinking as I > write... ...not executed with *current* code base. It might if we ever run > these things in separate threads. Better to fix this just in case. OK; also changed to '!server_state_pregame()' > - server_state_pregame() & server_state_started() function headers: In list > of states "and" -> "or" > - server_state_started() function header "startet" -> "started" corrected > - There's not enough context in diff to decide if this is correct or not: > srv_main.c:1698-> "NULL == pplayer || S_S_INITIAL != server_state()" > replaced with "NULL == pplayer || server_state_started()". They differ in > how they handle S_S_GENERATING_WAITING Here is the code block: --- start --- void handle_player_ready(struct player *requestor, int player_no, bool is_ready) { struct player *pplayer = player_by_number(player_no); bool old_ready; if (NULL == pplayer || !server_state_pregame()) { return; } --- stop --- I changed it to '!server_state_pregame()' > - You declare server_state_pregame() and server_state_started() as inline > functions in srv_main.h, yet you do not provide code for them there. Just > remove inline keyword. You may define them as macros instead (that would be > consistent with rest of the codebase) done > - More changes to how S_S_GENERATING_WAITING would be handled, but they all > seem ok I check all changes and adapted all (?) to include the correct check for S_S_GENERATING_WAITING. Thanks for looking at the patch (file #6588) _______________________________________________________ Additional Item Attachment: File name: 0001-add-new-server-state-S_S_SAVEGAME-to-prevent-changin.patch.diff Size:15 KB _______________________________________________________ Reply to this item at: <http://gna.org/bugs/?14230> _______________________________________________ Nachricht geschickt von/durch Gna! http://gna.org/ _______________________________________________ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev