Skip to content

[#1680] Update flight config stagemap according to unique stage ID #1688

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.

Sign up for GitHub

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

Jump to bottom
Merged
SiboVG merged 7 commits into from
Sep 27, 2022
Merged

[#1680] Update flight config stagemap according to unique stage ID #1688

SiboVG merged 7 commits into from
Sep 27, 2022

Conversation

@SiboVG
Copy link
Member

@SiboVG SiboVG commented Sep 22, 2022

This PR fixes #1680. Stages are now given a unique ID, just like flight configurations are assigned a unique ID (it's the same ID generation code). That unique ID is then used to identify stages when updating the stageMap in the flight configurations.

@hcraigmiller
Copy link
Collaborator

hcraigmiller commented Sep 23, 2022

Functions as expected, no anomalies found.

One observation. . . the active stages appear more muted than the inactive stages. Is this the desired behavior?

Stages

Build 1039
[Windows 11 Pro; Version 21H2; OS Build 22000.739; Windows Feature Experience Pack 1000.22000.739.0]
[Java "11.0.15" 2022-04-19 LTS; Java(TM) SE Runtime Environment 18.9 (build 11.0.15+8-LTS-149)]

@SiboVG
Copy link
Member Author

SiboVG commented Sep 23, 2022 •
edited
Loading

One observation. . . the active stages appear more muted than the inactive stages. Is this the desired behavior?

I guess that's just how your Windows systems renders toggled and untoggled buttons, should have nothing to do with this PR.

@SiboVG SiboVG marked this pull request as draft September 24, 2022 15:45
@SiboVG
Copy link
Member Author

SiboVG commented Sep 24, 2022

Whoops, I just now noticed that a RocketComponent (so also a stage) already has a unique ID generator. Will do a refactor of my code...

@SiboVG
Use built-in component ID instead of extra stageID (whoops)
3751b9c
@SiboVG
Fix issue in stage updating for sub-stages
b96087e 
updateStageNumbers would only update direct child stages of the rocket, instead of also ParallelStages
@SiboVG
Fix cheating unit test ;)
bbbf0be 
Booster D should have stage number 4, not 3 as it was previously
@SiboVG
Add unit tests for axial stage disabling
d9af735
@SiboVG SiboVG marked this pull request as ready for review September 24, 2022 19:23
@SiboVG
Copy link
Member Author

SiboVG commented Sep 24, 2022 •
edited
Loading

Fixed now. While writing unit tests I also found an issue in updateStageNumbers() and updateStageMap() in Rocket.java where the stage number of substages (boosters) did not update correctly - which the unit test in ParallelStageTest.java conveniently masked. The power of using unit tests is proven once again :)

@hcraigmiller
Copy link
Collaborator

hcraigmiller commented Sep 25, 2022

Functions as expected, no anomalies found.

Build 1048
[Windows 11 Pro; Version 21H2; OS Build 22000.739; Windows Feature Experience Pack 1000.22000.739.0]
[Java "11.0.15" 2022-04-19 LTS; Java(TM) SE Runtime Environment 18.9 (build 11.0.15+8-LTS-149)]

@SiboVG SiboVG merged commit ac2f39e into openrocket:unstable Sep 27, 2022
@SiboVG SiboVG deleted the issue-1680 branch September 30, 2022 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Changing stage order in component tree does "not" change activeness of stages

2 participants

@SiboVG @hcraigmiller