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

Less hacky offhand #46

Merged
merged 38 commits into from
Jan 19, 2025
Merged

Less hacky offhand #46

merged 38 commits into from
Jan 19, 2025

Conversation

Lyfts
Copy link
Member

@Lyfts Lyfts commented Jan 15, 2025

WIP - better description might come later.
This is a bigger project that will attempt to make the offhand feel more like a true vanilla offhand. Allocates an actual inventory slot for the offhand and will also render a slot for it in the player inventory. Using an inventory slot for the offhand also enables us to completely remove all of the incredibly janky hotswaps and makes a lot more items usable in the offhand without having to add any special compat.

Huge amounts of duplicated vanilla code that had only small tweaks to allow for offhand has also been removed as we can just trick vanilla into doing the job for us.

Closes #44
Closes #41
Closes #28
Closes #39
Closes #33
Closes #37
Closes #43 (tested & works fine now)
Closes #32 (tested & works fine now)
#25 (tested & works fine now) Nevermind, they want to dual wield mana blasters and have them shoot alternatively. Not gonna be implemented in this pr.
Closes #36

Might also close the following but further testing is needed
#32
#20
#40 (this is almost certainly fixed, but I can't test it since the mod keeps crashing no matter what I try)

@Dream-Master Dream-Master requested a review from a team January 15, 2025 16:56
Copy link
Member

@Caedis Caedis left a comment

Choose a reason for hiding this comment

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

This is looking to be an amazing rewrite

@Lyfts
Copy link
Member Author

Lyfts commented Jan 16, 2025

Alright, I think this is ready for review. There will almost certainly be bugs with a rewrite like this but overall I believe it leaves the mod in a much better, cleaner and functional state.

@Lyfts Lyfts marked this pull request as ready for review January 16, 2025 23:26
@Lyfts Lyfts requested review from Caedis and a team January 16, 2025 23:26
@Caedis
Copy link
Member

Caedis commented Jan 16, 2025

creating a pre release to do some testing

Copy link

Warning: 2 uncommitted changes
pull request create failed: GraphQL: Head sha can't be blank, Base sha can't be blank, Head repository can't be blank, No commits between GTNewHorizons:unjankify and origin:GTNewHorizons-unjankify-spotless-fixes, Head ref must be a branch, not all refs are readable (createPullRequest)

@Caedis
Copy link
Member

Caedis commented Jan 19, 2025

leaving open to let Lyfts merge if they think its good to go

@Lyfts Lyfts enabled auto-merge (squash) January 19, 2025 09:26
@Lyfts Lyfts merged commit cd2ffb3 into master Jan 19, 2025
1 check passed
@Lyfts Lyfts deleted the unjankify branch January 19, 2025 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment