-
Notifications
You must be signed in to change notification settings - Fork 178
QtFRED Generalized Mission/Campaign Save #7045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
QtFRED Generalized Mission/Campaign Save #7045
Conversation
TheForce172
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missionsave is missing the Guardian Threshold code added in #6989 and potentially other QT Fred only additions. I'm afraid you need to go through QTFred missonsave and see whether there were any more QTFred only additions/changes.
I'm going to assume guardian threshold is the only thing missing as missionsave changes should always have been done to both files for exactly this reason. Given that up until now FRED was the source of truth, I feel safe assuming the guardian threshold code added in the QtFRED PR is the only outlier. EDIT: Yes, a quick browse of QtFRED's history shows the primary PR authors are myself and Goober who religiously update both files. Checking the other outlying authors as far back as 2023 also show they updated both places. So I'm very comfortable assuming #6989 is the only change needed to port back in. |
659cf5e to
9dff8a6
Compare
JohnAFernandez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should watch for bugs in mission and campaign save for a while, but otherwise, this is a sensible change. Thanks for taking a look too, theforce
|
Agreed. I'm pretty confident in the mission save part of this one as there were almost no changes to that code outside of Clang modernization. Campaign saving had more changes but the community stopped relying on that editor years ago so ymmv. That said for merging this close to an RC stage I could go either way. On one hand merging means less maintenance with conflicts as people work with the existing mission save files. On the other it could introduce bugs to a critical piece of code at a possibly bad time. I leave it up to SCP to decide. |
647809f to
e368514
Compare
|
Conflicts resolved. This now patches QtFRED's campaign editor to use the new campaign save method. |
21fe3b2 to
eda8ead
Compare
eda8ead to
7e2bc94
Compare
7e2bc94 to
d3295af
Compare
This moves FRED's missionsave.cpp into a common location so that QtFRED can take advantage of the well-tested code. With this in place we can be sure that QtFRED's mission output will be identical to FRED's.
I also took this opportunity to move campaign file saving into its own file and generalize how it works. It requires a little bit more overhead to call but should be UI/application agnostic now. The QtFRED campaign save call sites will be updated once this or #7030 is merged which should trigger a conflict in either PR.
There are a handful of other TODOs here which will be handled in follow-ups or as QtFRED work continues.
One minor upgrade is that campaign names now properly save XSTRs and campaign descriptions are now unlimited length. (The already parsed at a greater length than was allowed to save so this change makes round-tripping a campaign file through the editor work as expected.
EDIT: I should add that I tested round-tripping multiple missions and campaign files (including complex files from mods like BP, JAD, and BtA).
Fixes #7028