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 &

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
> 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
> of states "and" -> "or"
> - server_state_started() function header "startet" -> "started"


> - 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()) {

--- 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
> consistent with rest of the codebase)


> - More changes to how S_S_GENERATING_WAITING would be handled, but they
> seem ok

I check all changes and adapted all (?) to include the correct check for

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


Reply to this item at:


  Nachricht geschickt von/durch Gna!

Freeciv-dev mailing list

Reply via email to