Skip to content

Conversation

@capdevon
Copy link
Contributor

@capdevon capdevon commented Apr 15, 2024

This pull request addresses several issues with the MatParamTexture class:

  • Duplicate variable: The texture variable is redundant as it's already stored in the inherited value variable. This PR removes the unnecessary texture field.
  • Missing Javadoc: The class and its methods lacked proper documentation. This PR adds comprehensive Javadoc comments to improve code readability and maintainability.
  • Inconsistent null handling: The behavior of setValue methods regarding null values differed. This PR modifies setValue to allow null values, aligning it with the behavior of setTextureValue and the superclass's setValue.

Key improvements:

  • Code simplification: By removing the duplicate variable, the code becomes cleaner and easier to understand.
  • Improved documentation: Clear Javadoc comments explain the class, its purpose, and how to use its methods.
  • Consistent null handling: All setValue methods now allow null values, preventing unnecessary exceptions.

Overall, this PR enhances the MatParamTexture class by streamlining the code, providing proper documentation, and ensuring consistent behavior.

--Edit:
Instead of overriding setValue in MatParamTexture, we can leverage the existing behavior in the superclass. The superclass method likely performs type and value checking, making an override unnecessary in this case (see #1797).

…ll throws exception

This PR solves all the problems listed above.
@stephengold stephengold added this to the Future Release milestone May 8, 2024
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

@stephengold stephengold merged commit 1d20091 into jMonkeyEngine:master May 17, 2024
@capdevon capdevon deleted the capdevon-MatParamTexture branch May 19, 2024 09:22
@yaRnMcDonuts yaRnMcDonuts modified the milestones: Future Release, v3.8.0 Dec 24, 2024
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