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
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
