-
-
Notifications
You must be signed in to change notification settings - Fork 533
[fixes #2521] #2530
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
[fixes #2521] #2530
Conversation
…main rocket, including the sustainer
|
Thanks for the PR! It works well if the "All Stage" combobox item is selected, but selecting individual stages is now broken. Screen.Recording.2024-08-12.at.12.24.32.mp4 |
SiboVG
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.
Plotting individual stages on a multi-stage plot is now broken.
JoePfeiffer
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.
I'm curious why it was necessary to break the plot for the sustainer into multiple series -- my first thought looking at it was that it would be most straightforward to just keep track of the stages active on each data point, and either construct a String of the form "sustainer + booster" to pass to formatSampleTooltip(), or pass an array of Strings with the names of all of the currently active stages as elements of the array. So there must be something I'm missing here.
|
Joe, that is a good point. I went with the approach of creating new series because the generateToolTip() method currently takes a single series as its input. I wasn't sure if/how the active stages for each data point could be maintained in order to access that information when generating the tooltip text. So, I opted to use a series for each combination of active stages. |
|
Ah, OK, I'd forgotten that we can't pass an event list into generateToolTip() (it's an override, not something we have control over the parameters). |
|
@SiboVG, looks like it's all working now. Thoughts? |
|
Yup, everything works great. Thanks a bunch @NCoppola25 for your excellent PR, much appreciated! |
The implementation from PR openrocket#2530 had to be remade from scratch because of the refactoring. This implementation is also a bit more flexible and efficient
This PR fixes #2521 in which sim plot tooltips did not include the stage name for the main rocket. This included the sustainer stage and any boosters before they separated.
There were two main problems:
This was fixed by splitting the sustainer series into multiple series (for multi-stage rockets), and setting the description of each series to include all stages currently on the rocket at the specific time. The point(s) of separation were found for each stage, and this was used to split the series.
Additionally, since there are now more series than branches (for multi-stage rockets), the coloring mechanism had to be adjusted. The colors of the lines on the plot are now set based on the number of series, which is still a function of the number of branches.
Here is a jar file for testing.