[Freeciv-Dev] [bug #21346] Behaviour on saving without have_resources poorly defined
URL: http://gna.org/bugs/?21346 Summary: Behaviour on saving without have_resources poorly defined Project: Freeciv Submitted by: jtn Submitted on: Sat Dec 14 13:26:42 2013 Category: None Severity: 3 - Normal Priority: 5 - Normal Status: None Assigned to: None Originator Email: Open/Closed: Open Release: Discussion Lock: Any Operating System: Any Planned Release: ___ Details: Freeciv seems to be changing its mind over time about what the map.server.have_resources variable (and associated 'specials' savefile capability) controls. If have_resources is not set at save time, if I'm reading the code right: * In savegame.c (and S2_2), none of resources, specials, or bases are saved. (And similar behaviour on load.) * On S2_3/S2_4, resources and specials are not saved, but bases are. (And similarly for loading new-format savefiles.) * On S2_5, resources and non-road/river specials are not saved, but bases and roads/rivers are. Loading is similar (old savefiles are treated as above). ** (There's also a rivers_overlay wart in saving, but since it saves S_OLD_RIVER from run-state I think it will never be activated in practice? I'm a bit surprised by the presence of compatibility code for absence of ruleset ROCO_RIVER, given that S_OLD_RIVER seems illegal while running now -- maybe this is work in progress?) * On trunk, resources are not saved, but extras (which includes old bases and specials) are. (all modulo rivers_overlay) This is I think mostly academic, because currently there's no way to save a scenario game without have_resources set, as it will get resources generated and have_resources set at game start, and the UI doesn't allow save scenario before that. (I think the debug-only '/scensave' command may be an exception to this?) If we take the older behaviour as more correct/intended, then I think the 'have_resources'/'specials' flags are both misnamed, as in fact they controlled all terrain additions other than rivers. Alternatively, we could retrospectively declare that it refers to *just* resources, and that older saving behaviour was a bug -- probably the most useful thing it controls is whether random resources get generated fresh on scenario load. (Would need to work out what to do when loading scenarios from older eras, but because of the saving behaviour there shouldn't be many kicking around with specials that have previously been ignored on load.) I haven't tested much of this understanding, and may have misread the situation; clearly some thought has been going into updating this stuff recently for roads/extras. ___ Reply to this item at: http://gna.org/bugs/?21346 ___ 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 #21347] Ability to not save resources not exposed to scenario editor
Update of bug #21347 (project freeciv): Depends on: = bugs #21346 ___ Reply to this item at: http://gna.org/bugs/?21347 ___ 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 #21347] Ability to not save resources not exposed to scenario editor
URL: http://gna.org/bugs/?21347 Summary: Ability to not save resources not exposed to scenario editor Project: Freeciv Submitted by: jtn Submitted on: Sat Dec 14 13:27:03 2013 Category: None Severity: 3 - Normal Priority: 5 - Normal Status: None Assigned to: None Originator Email: Open/Closed: Open Release: Discussion Lock: Any Operating System: Any Planned Release: ___ Details: For scenarios, the savefile format has the ability to specify that no resources be saved in the scenario, so that resources will be generated randomly when the scenario is loaded for play, according to settings chosen in pregame. (This is controlled by the specials capability in the savefile, which becomes the server's have_resources variable -- in fact these have controlled whether more stuff is saved at various times, see bug #21346 for confusion, which will need sorting out first.) * There's an additional wrinkle: if the savegame doesn't have the specials capability but does have rivers_overlay, then just river specials are saved (in older versions; gen_roads makes this moot in newer versions). However, there's no way for people editing scenarios with the built-in map editor to access this functionality, though -- it's currently necessary to hack savefiles by hand, I think. (Maybe the old, separate editor could use these?) The only way to load a scenario for editing is to start a game with it (which will generate resources and set have_resources). If you go into edit mode and do save scenario, those resources will be saved in the game and new resources will not be generated when the resulting scenario is loaded for play. Existing scenarios with this property (search as earth-80x50-v3, which has rivers_overlay instead of specials) thus can't be edited without losing this property. The obvious place to expose this in the UI is as a flag alongside Save players in game/scenario properties. Implementation issues: * What should be the UI if a player does save scenario with this flag set but resources present on the map? The obvious one is to pop up a clear resources or unset flag choice, but the client/server interaction makes this hard. Unlike players, I'm uncomfortable with quietly discarding stuff in the savefile but leaving it visible in the editor -- too easy to lose work. * We'll need to squirrel away the 'have_resources' value from game load so that it can be reinstated on scenario save, because currently you can only edit having gone through a game-start transition, which will set the real have_resources flag. Could in principle expose 'have_rivers_overlay', but if we only get round to this in S2_5 or later (as seems likely) then I think the issue goes away (and so should rivers_overlay, probably). May as well expose 'have_huts' at the same time (once the confusion of bug #21345 is sorted out). ___ Reply to this item at: http://gna.org/bugs/?21347 ___ 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 #21346] Behaviour on saving without have_resources poorly defined
Follow-up Comment #1, bug #21346 (project freeciv): (There's also a rivers_overlay wart in saving, but since it I don't think server ever *saves* as rivers_overlay, it just loads (old) scenarios where it's set. What is missing here, is having savegame format support for such scenarios with rivers as gen-roads. saves S_OLD_RIVER from run-state I think it will never be activated in practice? I'm a bit surprised by the presence of compatibility code for absence of ruleset ROCO_RIVER, given that S_OLD_RIVER seems illegal while running now -- maybe this is work in progress?) All this seems to be related to loading (where rivers_overlay code is) being different from saving (where you thought it to be) ___ Reply to this item at: http://gna.org/bugs/?21346 ___ 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 #21348] server/edithand.c:handle_scenario_info() looks redundant
URL: http://gna.org/bugs/?21348 Summary: server/edithand.c:handle_scenario_info() looks redundant Project: Freeciv Submitted by: jtn Submitted on: Sat Dec 14 15:48:30 2013 Category: None Severity: 2 - Minor Priority: 5 - Normal Status: None Assigned to: None Originator Email: Open/Closed: Open Release: Discussion Lock: Any Operating System: Any Planned Release: 2.5.0,2.6.0 ___ Details: PACKET_SCENARIO_INFO is defined as a packet that can go both server-client and client-server, but never actually goes in the c-s direction in practice. Client edits to this information go in PACKET_EDIT_GAME instead. It's been this way since r15698 http://svn.gna.org/viewcvs/freeciv?revision=15698view=revision, RT #40229. I'm guessing that this was the original plan for the editor before PACKET_EDIT_GAME was added (r15701 http://svn.gna.org/viewcvs/freeciv?revision=15701view=revision, bug #13452) so now is vestigial. Bug #13452 does say this: I'm not sure if the duplicated work in handle_scenario _info() and handle_edit_game() is ideal. Sure if there were a specialized gui widget for setting scenario parameters it would have to use the scenario info packet, but I think the property editor is enough ___ Reply to this item at: http://gna.org/bugs/?21348 ___ 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 #21349] Handicaps pointer rather than contents copied on civil war = double free()
URL: http://gna.org/bugs/?21349 Summary: Handicaps pointer rather than contents copied on civil war = double free() Project: Freeciv Submitted by: jtn Submitted on: Sat Dec 14 16:03:50 2013 Category: ai Severity: 3 - Normal Priority: 5 - Normal Status: In Progress Assigned to: jtn Originator Email: Open/Closed: Open Release: trunk r23854 Discussion Lock: Any Operating System: Any Planned Release: 2.6.0 ___ Details: In split_player(), we have cplayer-ai_common.handicaps = pplayer-ai_common.handicaps; handicaps is a void* allocated by handicaps_init() and freed by handicaps_close(), so should be copied deeply rather than shallowly. (Caused by patch #4197, I think.) This manifested as an invalid free() on server shutdown. Presumably also a tiny memory leak. *** glibc detected *** ./server/freeciv-server: free(): invalid pointer: 0x03f8e880 *** === Backtrace: = /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f6971bbfb96] ./server/freeciv-server(handicaps_close+0x10)[0x45ce30] ./server/freeciv-server(server_remove_player+0x195)[0x49eee5] ./server/freeciv-server(server_game_free+0xbc)[0x43b95c] ./server/freeciv-server(server_quit+0x18)[0x43bb18] ./server/freeciv-server[0x44b0eb] ./server/freeciv-server(handle_chat_msg_req+0x45d)[0x4f12bd] ./server/freeciv-server(server_handle_packet+0x6d2)[0x4966d2] ./server/freeciv-server(server_packet_input+0xbb)[0x43946b] ./server/freeciv-server(server_sniff_all_input+0x8b2)[0x4d7632] ./server/freeciv-server(srv_main+0xaa5)[0x43c625] ./server/freeciv-server(main+0x78a)[0x43405a] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7f6971b6276d] ./server/freeciv-server[0x4346b1] ___ Reply to this item at: http://gna.org/bugs/?21349 ___ 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 #21349] Handicaps pointer rather than contents copied on civil war = double free()
Update of bug #21349 (project freeciv): Status: In Progress = Ready For Test ___ Additional Item Attachment: File name: trunk-handicaps-copy.patch Size:2 KB ___ Reply to this item at: http://gna.org/bugs/?21349 ___ 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 #21349] Handicaps pointer rather than contents copied on civil war = double free()
Follow-up Comment #1, bug #21349 (project freeciv): Why are handicaps copied there at all? Shouldn't it be function of ai difficulty level? Given that difficulty level of the created player can be different from original one, this even sounds like a bug. ___ Reply to this item at: http://gna.org/bugs/?21349 ___ 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 #21349] Handicaps pointer rather than contents copied on civil war = double free()
Update of bug #21349 (project freeciv): Status: Ready For Test = None Assigned to: jtn = None ___ Follow-up Comment #2: Hm, I wasn't really thinking about the wider context. Indeed, this wouldn't be appropriate if you wanted a different AI type for civil-war players. I'll leave this to someone else (or me if I ever get round to it). ___ Reply to this item at: http://gna.org/bugs/?21349 ___ 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 #21349] Handicaps pointer rather than contents copied on civil war = double free()
Follow-up Comment #3, bug #21349 (project freeciv): Indeed, this wouldn't be appropriate if you wanted a different AI type for civil-war players. Not only if you want different AI type (actually that does not make difference at the moment as handicap setting is not yet AI type dependant), but even if you have different difficulty level between default AI type players, i.e. if player has level different from what gets assigned by default to created player. I'll leave this to someone else (or me if I ever get round to it). I have not tested, but I think it's as simple as leaving that line out completely as call to set_ai_level_direct() a couple of lines later should do the right thing (and yes, actually it should override what that erronous line is doing so we don't have end-user visible bug at the moment) ___ Reply to this item at: http://gna.org/bugs/?21349 ___ 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 #21268] Scenario start positions can't restrict nation choice for players
Update of bug #21268 (project freeciv): Status:None = Ready For Test Assigned to:None = jtn Planned Release: = 2.5.0,2.6.0 Summary: Scenario start positions don't restrict nation choice for players = Scenario start positions can't restrict nation choice for players ___ Follow-up Comment #2: The current behaviour has been there so long that scenario authors / players might be surprised by its suddenly starting to happen in a stable series. Attached patch for S2_5 and later makes it a scenario option, with restrictions off by default, and doesn't change any supplied scenarios, so they remain unrestricted. So no surprises. If there are ancient scenarios kicking around where this is desired, they'd have to be updated to a more recent savegame format and have the 'startpos_nations' option set. (With restrictions in force, the Gtk client's pick-a-nation dialog has groups which are entirely empty, which looks a bit naff; patch #3448 will take care of this.) This patch also mops up some of the comment changes about nation selection currently in file #19349 attached to patch #4304. (file #19476) ___ Additional Item Attachment: File name: trunk-S2_5-startpos-restricted-nations.patch Size:18 KB ___ Reply to this item at: http://gna.org/bugs/?21268 ___ 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 #21268] Scenario start positions can't restrict nation choice for players
Follow-up Comment #3, bug #21268 (project freeciv): (The label for this in the editor, Nation Start Positions, leaves a lot to be desired; unfortunately it needs to be short. Should really dust off pepeto's editor tooltips patch from bug #15580.) ___ Reply to this item at: http://gna.org/bugs/?21268 ___ 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 #21349] Handicaps pointer rather than contents copied on civil war = double free()
Follow-up Comment #4, bug #21349 (project freeciv): I have not tested, but I think it's as simple as leaving that line out completely as call to set_ai_level_direct() a couple of lines later should do the right thing Looks plausible (but I haven't tested either)... (and yes, actually it should override what that erronous line is doing so we don't have end-user visible bug at the moment) ...unfortunately we do have a bug on trunk, I think, as it will change the handicaps of *both* players. (I agree this saves S2_5 and S2_4.) ___ Reply to this item at: http://gna.org/bugs/?21349 ___ 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] [patch #4337] Worker thread for freeciv-mp-qt
URL: http://gna.org/patch/?4337 Summary: Worker thread for freeciv-mp-qt Project: Freeciv Submitted by: cazfi Submitted on: Sat 14 Dec 2013 11:44:47 PM EET Category: module installer Priority: 5 - Normal Status: Ready For Test Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: 2.5.0, 2.6.0 ___ Details: Make Qt modpack installer download installing of modpacks to happen in separate thread so that gui won't freeze while modpack is being downloaded. ___ File Attachments: --- Date: Sat 14 Dec 2013 11:44:47 PM EET Name: QtModpackWorker.patch Size: 6kB By: cazfi http://gna.org/patch/download.php?file_id=19477 ___ Reply to this item at: http://gna.org/patch/?4337 ___ 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] [patch #4329] Reimplement special name functions to savecompat
Update of patch #4329 (project freeciv): Status: Ready For Test = Done Assigned to:None = cazfi Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?4329 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
