Allow extensions to define units of measurement for their properties #4576
Conversation
D8H
commented
Nov 23, 2022
•
D8H
commented
- It use the scientific notation because it's the easiest to generate and it looks clean.
- It only works for behavior properties. It will be hard to do for objects like the ParticleEmitter because there is no property metadata.
- It doesn't allow event-based extensions to define units. I want to wait that everyone is ok with what is done for the built-in extensions because event-based extensions need a serialization and it's hard to go back.
D8H
changed the title
| // Strings are used instead of gdMeasurementUnit | ||
| // because the C++ instance would be dead when the component is rendered. | ||
| type Props = {| | ||
| label: string, | ||
| description: string, | ||
| elementsWithWords: string, | ||
| |}; |
There was a problem hiding this comment.
I'm not sure what "dead" means
There was a problem hiding this comment.
The gdMeasurementUnit instances are only living during their iteration on the properties when building the property editor view. As the rendering of MeasurementUnitDocumentation is done outside of this loop, the memory referenced by the gdMeasurementUnit instance has been reused and it displays wrong values.
I pass strings instead of gdMeasurementUnit to avoid this.
There was a problem hiding this comment.
ok then! (maybe adding what you wrote in the code would be a better comment 👍)
D8H
force-pushed
the
unit-of-measurement
branch
from
08c9b6d to
8b6e357
Compare
| // Strings are used instead of gdMeasurementUnit | ||
| // because the C++ instance would be dead when the component is rendered. | ||
| type Props = {| | ||
| label: string, | ||
| description: string, | ||
| elementsWithWords: string, | ||
| |}; |
There was a problem hiding this comment.
I'm not sure what "dead" means
There was a problem hiding this comment.
Looks good generally but the translations won't work when you switch to another language because things are constructed statically.
Would be worth having a "gd::MeasurementUnit::ApplyTranslation" to changes all the transaltable labels.
Note that it is a good thing to avoid recreating at every call a new MeasurementUnit! Otherwise this creates a lot of temporary just to destroy them after and this is needlessly costly.
But the labels should be updated when the language is changed in the editor.
|
I did the change, it seems to work.
There is a bit of duplicated code but I looked for static blocks in C++ and it seems complicated and risky. |
Yes. Terribly risky (there is even a page about it: https://en.cppreference.com/w/cpp/language/siof). Let's avoid doing anything risky in the statics and just keep a good old function applying translations. |
