Source Code Project Mantis - FSSCP
View Issue Details
0001953FSSCPFREDpublic2009-07-06 18:452010-04-13 10:39
ReporterKeldorKatarn 
Assigned ToGoober5000 
PriorityhighSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformIBM PCOSMS WindowsOS VersionVista SP1
Product Version3.6.11 
Target Version3.6.12Fixed in Version3.6.12 
Summary0001953: Departure locations and anchors are destroyed by the ship editor dialogue
Descriptionhere's what happens:

I have several fighters in wings and one fighter without a wing.

I select all of those.

Hat results in an intialization of the ship editor dialogue:

[code]fred2_open_3_6_11d.exe!CShipEditorDlg::initialize_data(int full_update=1) Line 583 C++[/code]

Now.. this one initalizes the dialogue, and among other stuff it does this check:

[code]if (Ships[i].wingnum < 0) {[/code]

If this holds true (ship is not part of a wing) it does (among other stuff) this:

[code]m_departure_location = Ships[i].departure_location;[/code]

Since this is done for all selected ships, it finds my one fighter which is not part of a wing and hence initializes m_departure_location with the departure location of that ship (in my case a docking bay).

Since all other ships are part of a wing, this is not changed after that, so m_departure_location remains 1 (1 = docking bay)

later on the ships are updated

[code] fred2_open_3_6_11d.exe!CShipEditorDlg::update_ship(int ship=20) Line 1232 + 0x32 Bytes C++
     fred2_open_3_6_11d.exe!CShipEditorDlg::update_data(int redraw=0) Line 1024 C++
     fred2_open_3_6_11d.exe!update_dialog_boxes() Line 1267 + 0xc Bytes C++[/code]

And inside there, this happens:

[code] if (m_departure_location != -1)
        MODIFY(Ships[ship].departure_location, m_departure_location);[/code]

So every single ship is set to what now is m_departure_location. Including the ships in a wing... you see the problem. Now ships have a departure location that doesn't even fit them.

I'm not exactly sure how to handle this, since this is probably not the only thing that can go wrong here.
Updating a dialogue with a departure location when multiple ships are selected... that just HAS to overwrite many many departure locations if not all ships have the same one...

I'm not sure how this can be fixed. I don't even know why all this initialization and updating is called...

I mean that a selection updates a dilogue is ok.. I mean the dialogue could be open and needs to display the data of the new selected ships

But why does updating the dialogue write back data to ships? That should only happen when I close the dialogue and confirm what I entered. Not before a dialogue was ever opened for the first time just because I select some ships...
Steps To ReproduceAnother way to reproduce:

- Create two fighters and one ship with a docking bay.
- Give one fighter the docking bay as departure location and the capship as departure anchor. The other fighter keeps hyperspace as departure target
- Now select both fighters.
- Open the ships dialogue and close it again
TagsNo tags attached.
has duplicate 0002096closed  Setting departure cue to docking bay causes ships in wings to have invalid departure target 
Attached Files

Notes
(0011231)
chief1983   
2009-11-09 22:31   
This is FRED so I'm throwing it Kara's way. It's a Saga showstopper so it needs some lovin'.
(0011395)
KeldorKatarn   
2009-12-10 01:51   
Any news on this? Last time I checked (Nightly Build 20091125_r5686) this still happened.
(0011601)
karajorma   
2010-01-29 05:14   
Well this is definitely annoying and it needs to be fixed but I don't see how it's a show stopper.
(0011602)
KeldorKatarn   
2010-01-29 06:12   
Because we're still editing and finalizing missions and this can accidently destroy missions during beta testing and introduce bugs that we might not find before release. Having a FRED build that potentially destroys missions if you just open the ship dialogue with a seletion that you haven't thought about twice is very problematic.
Call it what you want but it should definately be fixed in time for 3.6.12 RC.
We need it fixed by the time out beta test starts to ensure the correctness of our missions.
(0011603)
chief1983   
2010-01-29 10:30   
You do version your missions and compare diffs before committing changes right? That tends to catch those kinds of issues. Not saying it's optimal but at least you won't destroy anything irreparably.
(0011610)
karajorma   
2010-02-01 04:16   
To be honest I'm baffled about this one. This is a V bug not an SCP one but the arrival target does not show this behaviour and try as I might I really can't figure out why the hell not as the code is virtually identical for the two.
(0011611)
KeldorKatarn   
2010-02-01 06:49   
(Last edited: 2010-02-01 06:50)
Yeah. I tried to provide a fix myself when I found this. You have to step through the code step by step to find this crap.
I found exacty WHERE it happens but I didn't (and don't unfortunately) have the time to find out WHY it is happening. The code at that position is really strange. I've never seen code for opening a dialogue before that actually writes data.
But then again I've never seen code like FRED anyway. It's a real mess at some places.
I'll see if I can dig into this further later this week or so if I can make the time. This is really hard to track down. That dialogue might need some serious overhaul in places.
You're saying this has been around since Retail? Strange. Maybe some other change triggered the behavior? I never noticed it before.

(0011653)
karajorma   
2010-02-12 01:47   
It's definitely retail behaviour. I fired up FRED2 and tested it. The exact same circumstances will result in the data being wiped there too.
(0011878)
Goober5000   
2010-04-13 02:54   
A very sneaky bug! Fixed in revision 6051 (6052 in 3.6.12 branch). The fix will be in 3.6.12 final.

Thanks to Keldor's "steps to reproduce" which were very helpful in allowing me to pinpoint the problem. :)

Issue History
2009-07-06 18:45KeldorKatarnNew Issue
2009-11-09 22:31chief1983Note Added: 0011231
2009-11-09 22:31chief1983Assigned To => karajorma
2009-11-09 22:31chief1983Prioritynormal => high
2009-11-09 22:31chief1983Statusnew => assigned
2009-11-09 22:31chief1983Target Version => 3.6.12 RC1
2009-12-10 01:51KeldorKatarnNote Added: 0011395
2010-01-24 17:27Goober5000Relationship addedhas duplicate 0002096
2010-01-29 05:14karajormaNote Added: 0011601
2010-01-29 06:12KeldorKatarnNote Added: 0011602
2010-01-29 10:30chief1983Note Added: 0011603
2010-02-01 04:16karajormaNote Added: 0011610
2010-02-01 06:49KeldorKatarnNote Added: 0011611
2010-02-01 06:50KeldorKatarnNote Edited: 0011611
2010-02-12 01:47karajormaNote Added: 0011653
2010-04-13 02:54Goober5000Note Added: 0011878
2010-04-13 02:54Goober5000Assigned Tokarajorma => Goober5000
2010-04-13 02:54Goober5000Statusassigned => resolved
2010-04-13 02:54Goober5000Resolutionopen => fixed
2010-04-13 02:54Goober5000Target Version3.6.12 RC1 =>
2010-04-13 10:39chief1983Fixed in Version => 3.6.12
2010-04-13 10:39chief1983Target Version => 3.6.12