Skip to content

Conversation

@NCoppola25
Copy link
Contributor

@NCoppola25 NCoppola25 commented Aug 12, 2024

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:

  1. When setting the description for the sustainer series, its corresponding branch name was not used, so only the data type and units were passed to the tooltip.
  2. The main rocket's branch was modeled as the sustainer stage, but for multi-stage rockets, one or more boosters will be attached to the rocket for a portion of the flight. Therefore, the different portions of the flight needed to be labeled with all stages still attached to the rocket.

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.

@SiboVG
Copy link
Member

SiboVG commented Aug 12, 2024

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

Copy link
Member

@SiboVG SiboVG left a 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.

Copy link
Contributor

@JoePfeiffer JoePfeiffer left a 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.

@NCoppola25
Copy link
Contributor Author

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.

@JoePfeiffer
Copy link
Contributor

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).

@JoePfeiffer
Copy link
Contributor

@SiboVG, looks like it's all working now. Thoughts?

@SiboVG
Copy link
Member

SiboVG commented Aug 18, 2024

Yup, everything works great. Thanks a bunch @NCoppola25 for your excellent PR, much appreciated!

@SiboVG SiboVG merged commit 29598ef into openrocket:unstable Aug 18, 2024
SiboVG added a commit to SiboVG/openrocket that referenced this pull request Aug 26, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sim Plot tooltips do not show name of "sustainer" stage

3 participants