Skip to content

Conversation

@SiboVG
Copy link
Member

@SiboVG SiboVG commented Sep 8, 2022

This PR fixes #1643 and unit rounds the diameter labels.

Screenshot 2022-09-08 at 23 28 56

@hcraigmiller
Copy link
Collaborator

hcraigmiller commented Sep 8, 2022

As to the issue reported, functions as expected, no anomalies found.

As long as the issue of motor diameter units is being considered, the unit "meters" either needs to be disallowed or its decimal placement adjusted by two places. My preference would be disallowed.

Motor Diameter Meters

This issue, or a new issue?

Build 1004
[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 10, 2022

As to the issue reported, functions as expected, no anomalies found.

As long as the issue of motor diameter units is being considered, the unit "meters" either needs to be disallowed or its decimal placement adjusted by two places. My preference would be disallowed.

Motor Diameter Meters

This issue, or a new issue?

Build 1004 [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)]

Hm... for now I would leave it as it, but I'm wondering why the hell there are even 2 types of limit sliders? The slider for the diameter limiting and length limiting is different. I actually like that the length limit slider has explicit text boxes to set the limit values, is also useful for your mentioned case for meter units, so that it's clear what the limit values are even though the labels say '0'. Now there's the choice: do we add tick labels for both the length and diameter limit sliders, or do we remove the tick labels all together, for the diameter limit slider?

@hcraigmiller
Copy link
Collaborator

Although I regularly use the diameter slider and its tick marks with labels, I must admit, I have never found a need to use the length slider.

Comparing the two, however, there is one stark difference, motors have specific fixed diameters, and having the slider snap to those diameters is very helpful. On the other hand, there is no fixed length for a motor mount, the tube is whatever length the builder chooses (or has lying around), and a snap feature is not needed or probably even desired.

Against this backdrop, it would probably be helpful to some users to have the tick marks on the length slider labeled with a length measurement, but (as is currently) without the snap; I would prefer that the diameter tick marks and labels remain.

@JoePfeiffer
Copy link
Contributor

Although I regularly use the diameter slider and its tick marks with labels, I must admit, I have never found a need to use the length slider.
I have, when you get to larger motors at some point you decide to limit the length of the fin section at some reasonable length and there are absurdly long motors out there (particularly hybrids).

That said, I don't think I've ever checked the "limit to motor mount length" box. For small motors I'll typically have a motor block in there that limits the motor length to something shorter than the motor mount, and larger motors typically extend out the top of the motor mount tube.

Comparing the two, however, there is one stark difference, motors have specific fixed diameters, and having the slider snap to those diameters is very helpful. On the other hand, there is no fixed length for a motor mount, the tube is whatever length the builder chooses (or has lying around), and a snap feature is not needed or probably even desired.

Not only that, but the motor may not even match the length of the mount (see my comment above). So a snap really doesn't make sense.

Against this backdrop, it would probably be helpful to some users to have the tick marks on the length slider labeled with a length measurement, but (as is currently) without the snap; I would prefer that the diameter tick marks and labels remain.
The labels definitely need to remain.

@SiboVG SiboVG marked this pull request as draft September 16, 2022 00:15
@SiboVG SiboVG marked this pull request as ready for review September 20, 2022 23:58
@SiboVG
Copy link
Member Author

SiboVG commented Sep 21, 2022

Okay, I edited the code so that the labels are rounded with two decimals, but the leading zero is removed (which fixes the issue for the meter units):
Oi

@hcraigmiller
Copy link
Collaborator

Functions as expected, no anomalies found.

Build 1038
[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 22, 2022

Latest commit removes the tick marks for the length slider. This makes it clear that it is a free slider, and not a snap-point slider like for the diameter and total impulse. (because I admit that I was confused that the diameter slider was a snapping slider, and not a free slider like the length slider, so now there is a more clear distinction between the slider behavior)
image

@hcraigmiller
Copy link
Collaborator

Functions as expected, no anomalies found.

Build 1041
[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 db0e272 into openrocket:unstable Sep 23, 2022
@SiboVG SiboVG deleted the issue-1643 branch September 30, 2022 12:27
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.

Imperial motor dimension labels are unintelligible

3 participants