[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Status:None = Ready For Test Planned Release: 2.4.3, 2.5.0, 2.6.0 = 2.5.0, 2.6.0 ___ Follow-up Comment #9: Patches based on cazfi's attached, for trunk/S2_5. Main change is to conditionalise on was_game_started() rather than S_S_INITIAL, as the former is the right answer in the case of loading a started/running game into a fresh server (which will sit in S_S_INITIAL until everyone is ready). It fixes the original reported issue, although it's now necessary to dig out tileset-demo.sav from S2_3 or thereabouts to reproduce it. Haven't looked at whether anything needs doing for S2_4 yet. I think the only issue [on S2_4] is in loading then saving a pregame savegame (which is somewhat theoretical)? Does it really require loading old game first? Isn't there a problem even if one /create a player (maybe even having aifill != 0 suffice?) and then saves in pregame? Use-case for that would be creation of new scenario, one where is no map but all the other settings, including players, set. Updating such a scenario would be use-case for load + save, of course. I'm not sure it's currently possible to save a game that's not yet started but with players defined? I've not found a way to do so. (file #20067, file #20068) ___ Additional Item Attachment: File name: trunk-audit-color-assignment.patch Size:5 KB File name: S2_5-audit-color-assignment.patch Size:5 KB ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Follow-up Comment #6, bug #21126 (project freeciv): Digging into how I managed to make so many mistakes: I think I made this much worse on S2_5+ in patch #3443 (nation colours); before that (and hence on current S2_4), server_create_player() assigns a colour if passed NULL outside of pregame (so the comment referred to in comment #2 used to be more correct); afterward, it became the caller's responsibility. My mistakes were (a) not updating the comment (b) not checking all callers thoroughly enough (particularly savegame loading). * (Loading many S2_3 games into say S2_5 causes no obvious difficulty, which surprised me. Turns out the colorlessness of players persists indefinitely in the server, but with the default plrcolormode, the client gets the 'virtual' colors from player_get_preferred_color(), so all seems to be well until a save is attempted. Perhaps adding an assertion in package_player_info() based on server state would be a good idea.) Since S2_4 has different invariants, I think any patch for that branch probably needs to be a different shape. I think the bug is less severe there, too -- loading 2.3 savegames is fine, so I think the only issue is in loading then saving a pregame savegame (which is somewhat theoretical)? If so, perhaps we should put this cleanup back to 2.4.3? Also: in the current patch, in sg_save_player_main(), the sense of the Colorless player outside pregame test is wrong (generates errors when saving a valid game). ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Planned Release: 2.4.2, 2.5.0, 2.6.0 = 2.4.3, 2.5.0, 2.6.0 ___ Follow-up Comment #7: perhaps we should put this cleanup back to 2.4.3 Since you made it clear that it's not obviously correct, we shouldn't risk 2.4.2 with it, true. Even if we deem it more than cleanup (sse below), it's not reggression nor high severity either. I think the only issue is in loading then saving a pregame savegame (which is somewhat theoretical)? Does it really require loading old game first? Isn't there a problem even if one /create a player (maybe even having aifill != 0 suffice?) and then saves in pregame? Use-case for that would be creation of new scenario, one where is no map but all the other settings, including players, set. Updating such a scenario would be use-case for load + save, of course. ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Status: Ready For Test = None Assigned to:None = jtn ___ Follow-up Comment #8: Giving to one who knows the code in question. ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Status: In Progress = Ready For Test ___ Follow-up Comment #5: Attached patches - Allow colorless players to be saved in pregame - Assign players when loading old savegames that have already been started but lack player colors I don't think there's any reason not to have it in this form in S2_4 despite format freeze - no matter what we do, older revisions from 2.4.x are buggy with savegames without player colors, and such savegames already exist from 2.3. (file #19841, file #19842) ___ Additional Item Attachment: File name: AllowColorlessPregame.patchSize:3 KB File name: AllowColorlessPregame-S2_5.patch Size:2 KB ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Planned Release: 2.4.3, 2.5.0, 2.6.0 = 2.4.2, 2.5.0, 2.6.0 ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Follow-up Comment #3, bug #21126 (project freeciv): Now I wonder if similar thing happens when ever (not only when loading them from saved game) there's players in pregame and one tries to save. Then we need always to assign color to each player as soon as (s)he is created, one by one? In that case it might be better to just allow colorless players in pregame savegames. ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Update of bug #21126 (project freeciv): Category:None = general Status:None = Ready For Test Planned Release: = 2.4.3, 2.5.0, 2.6.0 ___ Follow-up Comment #2: /* If game is already running, server_create_player() will assign * a color, else colors will be assigned on game start */ Comment is wrong, by the way. When the savegame had no colour information, server_create_player() will be called with NULL colour so it won't be assigned. Fix to entire bug attached. As there's no scensave command in 2.4, the bug is rather theoretical there - AFAICS nothing requires colours before start. Postponing until after 2.4.2 has been safely released. (file #19814) ___ Additional Item Attachment: File name: AssignSavegamePlrColours.patch Size:1 KB ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
Follow-up Comment #1, bug #21126 (project freeciv):
I assume this code in savegame2.c to be relevant:
/* Get player color */
if (!rgbcolor_load(loading-file, prgbcolor, player%d.color,
pslot_id)) {
log_verbose(No color defined for player %d., pslot_id);
/* If game is already running, server_create_player() will assign
* a color, else colors will be assigned on game start */
}
1) Savegame is so old it has no player color information
2) It's not already running game
3) It's not started before we tried to /scensave
___
Reply to this item at:
http://gna.org/bugs/?21126
___
Message sent via/by Gna!
http://gna.org/
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #21126] assertion 'prgbcolor != ((void *)0)' failed.
URL: http://gna.org/bugs/?21126 Summary: assertion 'prgbcolor != ((void *)0)' failed. Project: Freeciv Submitted by: cazfi Submitted on: Tue 10 Sep 2013 08:42:01 PM EEST Category: None Severity: 3 - Normal Priority: 5 - Normal Status: None Assigned to: None Originator Email: Open/Closed: Open Release: Discussion Lock: Any Operating System: None Planned Release: ___ Details: In S2_5: ./fcser -f ../src/data/scenarios/tileset-demo.sav ... scensave tileset-demo results in a number of failed asserts in rgbcolor_save() [../src/common/rgbcolor.c::128]: assertion 'prgbcolor != ((void *)0)' failed. 2: Backtrace: 2: 0: ./server/freeciv-server() [0x6081ed] 2: 1: ./server/freeciv-server(vdo_log+0x89) [0x60ba59] 2: 2: ./server/freeciv-server(do_log+0x7d) [0x60bb0d] 2: 3: ./server/freeciv-server(fc_assert_fail+0x9f) [0x60bd3f] 2: 4: ./server/freeciv-server(rgbcolor_save+0x145) [0x5f3fa5] 2: 5: ./server/freeciv-server() [0x4bb029] 2: 6: ./server/freeciv-server() [0x4c97c6] 2: 7: ./server/freeciv-server(savegame2_save+0x65) [0x4cc375] 2: 8: ./server/freeciv-server(save_game+0xe0) [0x435920] 2: 9: ./server/freeciv-server() [0x447cd1] 2:10: ./server/freeciv-server() [0x4cda00] 2:11: /lib/x86_64-linux-gnu/libreadline.so.6(rl_callback_read_char+0x11b) [0x7f0f4d0d97fb] 2:12: ./server/freeciv-server(server_sniff_all_input+0x111b) [0x4cf36b] 2:13: ./server/freeciv-server(srv_main+0x16d) [0x439d6d] 2:14: ./server/freeciv-server(main+0x21e) [0x43117e] 2:15: /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7f0f4c5fa995] 2:16: ./server/freeciv-server() [0x431b8f] ___ Reply to this item at: http://gna.org/bugs/?21126 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
