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

Refactor first-person mode and configuration logic in ModifierLayer class #120

Open
wants to merge 1 commit into
base: 1.21
Choose a base branch
from

Conversation

RazorPlay01
Copy link
Contributor

  • Introduced getFirstPersonModifierOrDefault() method to encapsulate the logic for retrieving the highest-priority modifier, prioritizing FirstPersonModifier instances.
  • Updated getFirstPersonMode(float tickDelta) to delegate the retrieval of modifiers and animations more cleanly.
  • Updated getFirstPersonConfiguration(float tickDelta) with a similar streamlined logic.
  • Added detailed Javadoc comments to explain the behavior and logic of the methods:
    • getFirstPersonMode(float tickDelta)
    • getFirstPersonConfiguration(float tickDelta)
    • getFirstPersonModifierOrDefault()
  • Improved code readability, modularity, and maintainability by reducing duplication and clarifying method responsibilities.

This refactoring ensures greater clarity for future developers and prepares the code base for possible extensions, as well as prioritizing the newly created FirstPersonModifier.

- Introduced `getFirstPersonModifierOrDefault()` method to encapsulate the logic for retrieving the highest-priority modifier, prioritizing `FirstPersonModifier` instances.
- Updated `getFirstPersonMode(float tickDelta)` to delegate the retrieval of modifiers and animations more cleanly.
- Updated `getFirstPersonConfiguration(float tickDelta)` with a similar streamlined logic.
- Added detailed Javadoc comments to explain the behavior and logic of the methods:
  - `getFirstPersonMode(float tickDelta)`
  - `getFirstPersonConfiguration(float tickDelta)`
  - `getFirstPersonModifierOrDefault()`
- Improved code readability, modularity, and maintainability by reducing duplication and clarifying method responsibilities.

This refactoring ensures greater clarity for future developers and prepares the code base for possible extensions, as well as prioritizing the newly created FirstPersonModifier.
@RazorPlay01
Copy link
Contributor Author

Well now I have modified a little bit the code that decides which configuration to use with priority giving it to FirstPersonModifier if it is present as I commented in the other pull request

@RazorPlay01 RazorPlay01 changed the title Refactor first-person mode and configuration logic Refactor first-person mode and configuration logic in ModifierLayer class Dec 15, 2024
@dima-dencep
Copy link
Contributor

You sent a pr where you added a new modifier but then it turned out to be overridden by a different animation and you decided to submit a more delusional pr?

You can override these methods in your container and it will be a more reliable option

@RazorPlay01
Copy link
Contributor Author

I know, but this is a more practical and easier way to use.

@ZigyTheBird
Copy link
Contributor

I don't oppose this PR as long as it doesn't make any API breaking changes (Which it seems to currently do, take a look at my review)

@RazorPlay01 RazorPlay01 marked this pull request as draft December 16, 2024 09:34
@RazorPlay01 RazorPlay01 marked this pull request as ready for review December 16, 2024 09:34
@RazorPlay01
Copy link
Contributor Author

It shouldn't break anything as it respects how it was done before as long as the new modifier is not used

@ZigyTheBird
Copy link
Contributor

Oh right, I didn't pay close enough attention.

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