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

feat: Added rendering of armor when using the custom first person #134

Open
wants to merge 4 commits into
base: 1.21
Choose a base branch
from

Conversation

RazorPlay01
Copy link
Contributor

A new FirstPersonMode has been added which allows the rendering of armors when using the first person custom

I have preferred to do it this way and not add each shoulder separately to FirstPersonConfiguration because I doubt that anyone would be interested in handling the rendering of the shoulder pads separately, this way it will only be visible according to the arms if both are visible their respective armor parts will be shown instead when only one is visible only that one will show its armor.

@ZigyTheBird
Copy link
Contributor

I think this would work better as a boolean in the FirstPersonConfiguration class rather than it's own first person mode, since I also plan to add my own first person mode and this would complicate things. (assuming my PR gets merged)
You can add a constructor to FirstPersonConfiguration without the new option for backwards compatibility.

@RazorPlay01
Copy link
Contributor Author

It's not a bad option when I free up a little I do it now I have too much work :")

@ZigyTheBird
Copy link
Contributor

Welp, I gave up on what I was trying to do... too hard :(

@ZigyTheBird
Copy link
Contributor

I wanted to make a good way to have vanilla-style first person anims, rather than just rendering the third person model in first person.

/**
* Use the 3rd person player model (only arms/items/shoulder armor) to render accurate first-person perspective
*/
THIRD_PERSON_MODEL_SP(true),
Copy link
Owner

Choose a reason for hiding this comment

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

Is this new enum entry really necessary?

Wouldn't it be better to have the old entry (with updated comment) and use firstPersonConfig to optionally enable armor/shoulder rendering.

@KosmX
Copy link
Owner

KosmX commented Jan 21, 2025

i'll just push directly to your repo
(On PRs, the original author has access to push into the fork PR branch as long as the fork is on a personal account, and the PR author doesn't say otherwise)

@ZigyTheBird
Copy link
Contributor

You should probably add a constructor to the first person configuration class with the original 4 booleans so mods don't break
Actually maybe write all the constructors by hand since that would be less confusing for beginners
Also I am looking for a fix to the lombok messing with the source code issue

@ZigyTheBird
Copy link
Contributor

Hope this gets merged soon, I am almost done updating my API just waiting on these changes

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