Crash #6581 moving object in production window

Problems and solutions for installing or running FreeOrion, including discussion of bugs if needed before posting a bug report on GitHub. For problems building from source, post in Compile.

Moderator: Oberlus

Message
Author
User avatar
MatGB
Creative Contributor
Posts: 3310
Joined: Fri Jun 28, 2013 11:45 pm

Re: Crash #6581 moving object in production window

#16 Post by MatGB »

See, I'd love that, but it's a Git diff file so Geoff can't commit it, chance of doing it in SVN? (the code itself is way beyond me, but thanks for the work).
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.

User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

Re: Crash #6581 moving object in production window

#17 Post by Dilvish »

PhilSophus, for an example popup menu in a similar context, take a look at ProdQueueListBox::ItemRightClicked in ProductionWnd.cpp
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

PhilSophus
Krill Swarm
Posts: 11
Joined: Thu May 16, 2013 9:52 am
Location: Germany

Re: Crash #6581 moving object in production window

#18 Post by PhilSophus »

Geoff the Medio wrote:Some range-checking on the iterator parameter to BuildItemRightClicked would probably be best before attempting to use what it points to.
I essentially copied it from BuildItemDoubleClicked. Only the passing of a queue_pos to outgoing signals was added. So, if there is anything missing, it was already missing before.
Geoff the Medio wrote:Rather than right click immediately enqueuing, I'd have it create a pop up menu with commands to enqueue at top or bottom.
Yes, maybe, but I didn't want to dig deeper.
Geoff the Medio wrote:
PhilSophus wrote:Phew, what a codebase.
Elaborate?
It looks overly complex. Decoupling parts is a good thing unless you overdo it in such a way, that even changing something trivial becomes a nightmare. I mean, adding a modifier key to a mouse click would have required to change a long chain of signals, some of which are used almost everywhere.

BTW: I noticed savegames became incompatible due to my change. Is there a savegame version identifier somewhere, that needs to bumped?

User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

Re: Crash #6581 moving object in production window

#19 Post by Dilvish »

PhilSophus wrote:BTW: I noticed savegames became incompatible due to my change. Is there a savegame version identifier somewhere, that needs to bumped?
I think that previously saved games became incompatible because you changed constructors in one of the back-end classes (ProductionQueueOrder), which the boost (de)serialization code notices and doesn't like; changing any of their attributes would do it also. Merely changing regular class methods, and any changes to the UI, should not render saved games incompatible. Still, even though it might have been possible to cobble together a pair of the existing ProductionQueueOrders to accomplish the same effect without rendering old saved games incompatible, I think your approach is probably better.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: Crash #6581 moving object in production window

#20 Post by Geoff the Medio »

PhilSophus wrote:So, if there is anything missing, it was already missing before.
Quite plausible... It'd still be better with safety checks.
Decoupling parts is a good thing unless you overdo it in such a way, that even changing something trivial becomes a nightmare. I mean, adding a modifier key to a mouse click would have required to change a long chain of signals, some of which are used almost everywhere.
Are you arguing for or against (more) decoupling?

Regardless, I'm clear on what you (might) suggest as an alternative to how things are structured now... I suppose signals could be initially written with lots of unused parameters to make them more future proof, but that would make things more complex, with not much benefit.

(I also question your definition of "almost everywhere"...)
BTW: I noticed savegames became incompatible due to my change. Is there a savegame version identifier somewhere, that needs to bumped?
No; some changes break saves. Updating a version number won't change that.

You can possibly rescue the save by loading it in the old version, ending the turn, and then before doing anything (in particular issuing any orders, ie. not modifying any queues), re-saving, and then trying to load that in the new version.

Post Reply