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

Make Vampirism compatible with newer and older Forge versions #1236

Merged
merged 2 commits into from
Jul 23, 2023

Conversation

Cheaterpaul
Copy link
Member

@Cheaterpaul Cheaterpaul commented Jul 19, 2023

As reasoning #1231 (comment) and #1235

We have multiple choices here:

  1. use Forge > 46.1.29
    • will work
    • forces the user to use newer forge versions
    • forces Vampirism to be removed / not added to modpacks
  2. restrict Vampirism to Forge [46.1.0, 46.1.3]
    • will work
    • user will complain why Vampirism does not support newer version
    • limits the users freedom
  3. try to do both
    • all forge version will be supported
    • the users can choose what to do
    • if someone complains / reports a bug we can always tell them to use Forge 46.1.3

While I do not like any of these choices I think the 3rd one is the best one given the circumstances.

This solution is based on Forge 46.1.0, keeps the old methods (we do not care about Forge [46.1.6-46.1.29)) and uses reflections to register the eye height event for newer versions

…6.1.0 and 46.1.29

this solves the problem that EntityEvent.EyeHeight might not be available on runtime and compile time by creating a proxy event consumer via reflections

close #1235
@Cheaterpaul Cheaterpaul self-assigned this Jul 19, 2023
@Cheaterpaul Cheaterpaul merged commit 28213be into dev Jul 23, 2023
1 check passed
@maxanier
Copy link
Member

I would always prefer to stick with recommended build. Anyone using most recent MinecraftForge can still use .beta.2.
Once the dust has settled we will migrate to NeoForge, so eventually this won't be an issue anymore.
I don't think we should actively support latest MinecraftForge

@maxanier
Copy link
Member

We should restrict the Forge version range then and maybe manually change the CurseForge Displaytitle to make things clear

@Cheaterpaul Cheaterpaul deleted the feature/entity_eye_size branch September 19, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants