Skip to content
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

Remove global mutable fields and reduce data-type related duplication #187

Merged
merged 2 commits into from
May 17, 2023

Conversation

kyay10
Copy link
Collaborator

@kyay10 kyay10 commented Apr 13, 2023

  • Use Vecx instead of FloatArray
  • Add Vec3Setter and Vec4Setter, function types that encode placing a value inside a VecX or FloatArray
  • Eliminate usage of global mutable fields like _f and _b and withX and instead use mutableReference/mutablePropertyAt
  • Get rid of previous erroneous MutableProperty implementation (the getter was constant, so it wouldn't reflect setter) and replace it with a factory function
  • Unify and rename widgetType-style functions (e.g. sliderInt) to reduce duplication
  • Add NumberOps interface along with associated methods to facilitate generic math operations
  • Remove dataTypeHelpers.kt and instead add them as functions on NumberOps
  • Remove PlotArray and replace it with an inline lambda
  • Change some KMutableProperty0 into plain values. This fixes a bug where min/max values were swapped by functions and affected the outside (Which is a misinterpretation of the C++ version. See this for an explanation of ImSwap behavior)
  • Extract common elements of dragN, inputN, and sliderN into a widgetN function.
  • Remove MutableProperty0.java and instead use MutableReference
  • Fix refCol usage in colorPicker4

For the future: Maybe we should also remove the overloads that take an array and a ptr and call mutablePropertyAt? It's useless duplication because the user can just call mutablePropertyAt on their own.

- Add Vec3Setter and Vec4Setter, function types that encode placing a value inside a VecX or FloatArray
- Eliminate usage of global mutable fields like _f and _b and withX and instead use mutableReference/mutablePropertyAt
- Get rid of previous erroneous MutableProperty implementation (the getter was constant, so it wouldn't reflect setter) and replace it with a factory function
- Unify and rename widgetType-style functions (e.g. sliderInt) to reduce duplication
- Add NumberOps interface along with associated methods to facilitate generic math operations
- Remove dataTypeHelpers.kt and instead add them as functions on NumberOps
- Remove PlotArray and replace it with an inline lambda
- Change some KMutableProperty0 into plain values. This fixes a bug where min/max values were swapped by functions and affected the outside (Which is a misinterpretation of the C++ version. See https://replit.com/@kyay10/ImSwap-doesnt-swap-the-value-inside-pointers for explanation of ImSwap behavior)
- Extract common elements of dragN, inputN, and sliderN into a widgetN function
@kyay10 kyay10 requested a review from elect86 April 13, 2023 07:26
@elect86 elect86 merged commit 9813d37 into kotlin-graphics:1.89.2 May 17, 2023
@elect86
Copy link
Collaborator

elect86 commented May 18, 2023

Thanks for the PR

For the future: Maybe we should also remove the overloads that take an array and a ptr and call mutablePropertyAt? It's useless duplication because the user can just call mutablePropertyAt on their own.

That actually makes sense..

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.

2 participants