search

chbergmann / OpticsWorkbench

GNU Lesser General Public License v3.0
69 stars 23 forks source link

Beam Nr Columns / Rows can be negative #6

Closed wohltat closed 1 year ago

wohltat commented 2 years ago

Beam Nr Columns / Rows can be negative, which of course makes no sense.

image

chbergmann commented 2 years ago

is this really a problem ?

wohltat commented 2 years ago

This is definitely not critical but it would make the interface nicer. I like to change values by scrolling the mouse wheel. If i want to choose "1", instead of carefully scroll down, i could just stroke the mouse wheel some times and still have mental capacity for other things. Good user interfaces make it hard to do errors and make it easy to do what you want.

I don't know how hard it is to implement though.

luzpaz commented 1 year ago

Out of curiosity, how would you modify the code to not allow negative properties ?

chbergmann commented 1 year ago

I am using PropertyQuantity for Beam NrColumns/Rows. Normally a quantity cannot be negative, but the FreeCAD GUI allows negative values. I do not find any better property type which I could use. The fix has to be made in the FreeCAD GUI.

luzpaz commented 1 year ago

Hi @wwmayer sorry to bother you. If you don't mind, there is a question here. Is there a condition that we can specify in the GUI that PropertyQuantity can't be a negative number?

wwmayer commented 1 year ago

Normally a quantity cannot be negative

Of course, a quantity can also be negative. It would make things very difficult if we disallowed any negative values.

Example: Consider a car that becomes faster and faster. It has a positive acceleration. If it's throttled then the acceleration will be negative.

The fix has to be made in the FreeCAD GUI.

You can use a PropertyQuantityConstraint instead where you can set its lower limit to 0. But when talking about rows and columns then I think a floating point number is the wrong type. The type PropertyIntegerConstraint looks more suitable to me.

luzpaz commented 1 year ago

Thanks so much for your time wmayer :pray:

chbergmann commented 1 year ago

Ok, I made wrong assumptions about what PropertyQuantity means. With PropertyIntegerConstraint it is possible to use only positive integer values. Thank you @luzpaz and @wwmayer for clarification. I changed the code and this issue can be closed.