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

VarType: format code #2199

Merged
merged 4 commits into from
Feb 29, 2024
Merged

Conversation

capdevon
Copy link
Contributor

@capdevon capdevon commented Feb 19, 2024

VarType.Boolean: reorder the list of javaTypes in accordance with Float and Int

  • format code
  • update License year
// before
Float("float", float.class, Float.class), 
Int("int", int.class, Integer.class), 
Boolean("bool", Boolean.class, boolean.class), //<----
// after
Float("float", float.class, Float.class), 
Int("int", int.class, Integer.class), 
Boolean("bool", boolean.class, Boolean.class), //<----

…at and Int

VarType.Boolean: reorder the list of javaTypes in accordance with Float and Int

- format code
- update License year
fix import com.jme3.shader.bufferobject.BufferObject
Update VarType.java
Copy link
Member

@stephengold stephengold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combining formatting and functional changes in a single PR obscures the functional changes and makes them much harder to find and review. Please don't do this again.

On close inspection, it looks like the UniformBufferObject and ShaderStorageBufferObject constants have been combined into a new BufferObject value. The rationale for this change is unclear.

I'm unsure whether this enum's ordinal value is used anywhere---for instance if it's serialized as part of a Savable. If it is, then I worry that change might break existing code.

Similarly, if existing code depends on the names of the changed constants, this might be a breaking change.

@capdevon capdevon changed the title VarType.Boolean: reorder the list of javaTypes VarType: format code Feb 19, 2024
@capdevon
Copy link
Contributor Author

capdevon commented Feb 19, 2024

Thanks for your review @stephengold , I agree with you. I noticed this discrepancy in the order of javaTypes while designing my editor. The correct order would have made my code more elegant, instead I will have to handle the problem in my code as usual. Now it is too late to fix past mistakes. I have restored the original code. This PR only formats the class code.

Copy link
Member

@stephengold stephengold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@scenemax3d scenemax3d added this to the v3.7.0 milestone Feb 29, 2024
@scenemax3d scenemax3d merged commit 80c2577 into jMonkeyEngine:master Feb 29, 2024
14 checks passed
@capdevon capdevon deleted the capdevon-VarType branch March 1, 2024 08:49
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.

3 participants