Closed Wolfdv1 closed 4 months ago
Up and running for fabric 1.21 at least, other platforms untested as of yet
Good news, there are no other platforms as of right now, unless you count 1.20.6/1.20.4/1.20.2 etc fabric.
There will be a bit more work to get everything ready for 1.21, for example, src/main/java/dev/tr7zw/itemswapper/ItemSwapperMod.java duplicates the code currently, but I already prepared a small utility method to get the resource location over all versions, so no duplication is needed (https://github.com/tr7zw/NotEnoughAnimations/blob/main/src/main/java/dev/tr7zw/notenoughanimations/logic/HeldItemHandler.java#L49 as seen for example here). I will check out the pr in more detail later, currently still bashing my head against issues over at WaveyCapes/NBTAPI, while trying to recover from work.
Ah I thought that may be the case- wasn't sure tho as forge/neoforge build scripts seem to be semi-implemented. Makes sense! Honestly that would make much more sense than how I got it running, I think that this PR could be closed if you want as I would say the way I got it running would only make things messier, having a version neutral implementation of ResourceLocation would be much better than the messy preprocessors that I used. The changes to blaze 3d are something that I wanna look into more also, as seems there may be better ways of using it now.
Thanks for the good work, hope your day goes well!
I spent the entire day building Ikea stuff, so that was something. The Forge/NeoForge stuff is there because the mod base is shared between all my mods, which also do a release for that, and at some point I do want to port ItemSwapper to Forge/NeoForge too. For the rendering I did start to also provide version-neutral methods inside the NMSHelper class(one of the things shared between all mods), but still stuck on porting some render lines of WaveyCapes, as color also completely changed(possibly the method is currently broken, as it changed from 0f-1f to 0i-255i?).
Also was about to say, why close the pr, but jeeez that's many locations creating ResourceLocations xD Probably will just fix it inside this pr. Another thing that needs attention is the networking. Right now its really broken, and I wasn't able to get help on the fabric discord. On 1.20.6 it just doesn't work right, and using the fabric API didn't work at all for <=1.20.4, so in these versions I just mixined into mc to implement it myself. Such a mess.
Ahh gotcha! I might look into your modbase, really cool approach for backwards compatible updates/feature adds! Haha Ikea stuff can be satisfying to build, like adult lego, but also infuriating sometimes so i feel u Yeah IDK why I committed to doing the resource locations manually, I started and was like 'ah a simple find and replace will work with some regex checks' and that quickly got outta hand- not ideal. Also indeed, the rendering stuff I believe is a reasonably big issue, I've been using my build but found that its not compatible with REI, which i also did an experimental update for- this is cuz the way I updated blaze3d use is for sure wrong, think mainly the big part is instantiating a new ByteBufferBuilder for every call and relying on garbage collection is.. shameful on my part ngl I didnt know that about the networking implm in the api.. frustrating
having quite the struggle getting the resource packs to be parsed and loaded, can't find a good place to register the resource loader helper class on the SimpleJsonResourceReloadListener, tried to see if fabric API has a better way of doing this but as far as I couldn't find would need a significant rewrite to the resource loader helper class- I might be missing something obvious tho
1.21 working nicely now, having some issues with all other versions to behave with the new resourcepack parsing system tho...
Sorry, got really distracted today after work by the crap going on with uBlockOrigin trying to block sponsor banners on modrinth/curseforge, breaking all kinds of stuff in the meantime. Good job fixing the loader, if it doesnt work with old versions, might be able to just have both options in the code, toggled on/off depending on mc version. Was the "default_with_axes_as_weapons" folder pushed by accident? That's what the the modify tags are for, or just overwriting the tools/weapons json in its own optional pack. Creating a copy of everything should never happen.
No apologies needed! Yeah I'm trying to figure out why it isn't liking the old versions, since it should be fairly version agnostic, indeed doing a toggle could work, tho the old version used a mixin and the new does not- I had trouble toggling on/off mixins with the preprocessor, likely was doing something wrong tho! (and could just leave the mixin in for new version, but then disable it via the IMixinConfigPlugin) and oops! must've blanket committed one time- apologies new to git (as you can probs tell), that was my OG resourcepack I put in for testing, so was made before I knew anything about how it works haha!
preprocessor toggles lines in the java files. So to add/remove the mixin, just fully encase the method or just the annotation inside the preprocessor tags. No need for IMixinConfigPlugin etc. Same way the new implementation not working on old mc can just have a //#if MC >= 12100 at the top and //#endif in the last line, just removing the entire file on older versions. So yea, was an accidental push :D
Done, all is looking good, all that is missing for 1.21 is potentially some work to figure out what classes as a 'Music disk' now, since they're now treated as a generic item and are given the music tracks via the data driven components, so figuring out what disks have been added by other mods, or even added during game play via a data pack may be tricky, it is working for vanilla disks tho as-is.
The only other thing (minus code cleanup) that I could see needing addressing is that in the 1.19.4 build, for some reason the hotbar quick switch for tools and such appears to not display the item icons, all other GUIs are working as normal, and it's only in 1.19.4- struggling a change I made that could've caused this, so could have been an existing issue? See below
(1.20.1/all versions except 1.19.4): vs (1.19.4):
Tested the latest release and indeed it is present in the current published 0.6.3 build, so isn't something new:
Hm, that bug should have been fixed by https://github.com/tr7zw/ItemSwapper/commit/ae19f5dc1ff6c223348c1503cd850a327a41cddd but it's weird in general, because the aspect ration/gui scale somehow sometimes caused this. The item is rendered behind the element, which shouldn't be linked to the amount of items/aspect ratio/gui scale at all.
Might have a play around with it when I get the chance- odd that it's only 1.19.2, will try check what has changed
Edit: btw I think it was having the z/blitOffset set on the old screen.blit method that was causing it, setting that to 0 (and only changing the z when posing) seems to fix it entirely
Okay all fixed and tested across versions :) 2ab49a8
Also added static 'layers' as found it necessary to debug z-axis render issues across versions with slightly different pose behavior.
It might be all ready now? Only thing that comes to mind is right now the system for music disks is still hardcoded, so no modded or data driven 'disk' support
Music discs are no longer hard coded, setup a mixin to capture items registered with the component that now makes a generic item a playable one :) wanted to see if could find a sneaky way of doing items with this componant made through commands be included at runtime, but the overhead of checking would be quite a bit- and didnt fancy making a new mixin to capture when such items are made during runtime. At least this approach should apply to any modded items that go through the vanilla items registration method. e7405f8
Apologies for the chain of WIP commits and such in this PR - I appreciate the input/insight and friendlness you've given and such @tr7zw. if I were to do similar again I probs would've waited until it's at the stage it is now to actually make the pr (likely with alot less commits) or use the draft pr tags- I'm new to git and contributing :)
Regardless finally feel the PR/fork is in a place I'm happy with!
Wow, got back from the festival I was at, just gave it a shot, from brief testing and checking the code, didn't see any major issues. Also just finished up the WaveyCapes 1.21 port, I clearly was overworked when I last worked on it and made some glaring mistakes. This pr saved me probably many hours, especially on the layer priority debugging and correcting the resource pack loading. I'll merge it as is(probably not squash it for better tracking why which change was done), do a final quick check and then publish it. The networking issues where some client + server combinations crash/don't work can wait I think compared to getting this out.
Spoke too soon, pre 1.20 Waveycapes is now broken and I can't figure out why 😵💫🙄
Happy to help! Hope you had a good time at the festival, which one was it?
Also I enjoyed working in the codebase, think you've got a nice toolchain/build approach for your mods and admire the support continuation for various versions - I'm on holiday myself atm but if waveycapes if still presenting issues when I get home; I'll have some free time to do sone testing n poking around. Working with Minecraft's rendering system does kinda give me a a headache ngl, but doing some version testing on waveycapes might get me more comfortable with it- even if it's all sorted by then (this weekend) think will give it a look.
Hope your week is going well!
Update for 1.21
Known Issues