search

ReplayMod / ReplayStudio

Library for handling Minecraft Replay Files (.mcpr files)
GNU Lesser General Public License v3.0
44 stars 14 forks source link

ViaVersion API integration #3

Closed Gerrygames closed 6 years ago

Gerrygames commented 7 years ago

Automatically convert old replays while reading them using the ViaVersion API.

Johni0702 commented 7 years ago

Please rebase this PR onto and change its target to the 1.9.4 branch. Development is happening on master (which is for 1.8) and then merged upwards. 1.9.4 is the first branch that would benefit from these changes.

While you're at it, please also fix your invalid committer email address.

Gerrygames commented 7 years ago

I just tried to do so, but Windows is crap. http://puu.sh/wquTf/a329d356ce.png

Johni0702 commented 7 years ago

You need to install Maven to be able to build anything older than 1.12 as those branches automatically build MCProtocolLib locally (the repo went offline). Edit: And run git submodule update --recursive --init

Gerrygames commented 7 years ago

Maven is installed on my machine and i ran the submodule update. http://puu.sh/wqwvk/a54448795b.png

Johni0702 commented 7 years ago

I finally got around to handling this PR and I'm currently in the process of backporting it to 1.8 and understanding it. So, I've got a few questions (possibly more to come):

Gerrygames commented 7 years ago

Thanks for looking into my code. I wrote the code a long time ago (even before implementing into your code) and do not exactly know what I was thinking while overwriting the first two things mentioned. I just tried removing the overwritten code and everything still works fine, so that's that. The chunk bulk packets require special treatment because one chunk bulk packet gets convertet into multiple chunk packets. Otherwise chunk bulk packets would just not work, as you can see here.

If you have any further question just ask me.

Johni0702 commented 7 years ago

Thanks for the quick answer, I'll leave those two overwrites out then.

I'd also like to find some way in which the chunk packets don't have to be special cased because there might be other packets (potentially in the future) that also require this special casing. They might also have to be remapped again in some later version so we'd have to push them through the pipeline again after the special casing and this is probably to get rather complicated rather quickly.

It seems like that packet remapper is indirectly calling UserConnection.sendRawPacket which currently is just a noop. Have you tried getting the transformed packets from there instead of the special casing and can tell me that this won't work? Otherwise I'm going to try it.

Gerrygames commented 7 years ago

Getting the packets from UserConnection.sendRawPacket should work fine. The only thing speaking against it, is that the packets would obviously not be returned by readPacket. But I do not know in how far this could be a problem.

Johni0702 commented 7 years ago

I've rebased this on 1.8, did some refactoring (mainly extracted the viaversion-related code from the ReplayInputStream into its own class and deprecated old methods instead of removing them to maintain binary compatibility for now), fixed some bossbar-related crashes and some other things.

Works great so far, thanks a bunch for getting this going. I've pushed my changes to the viaversion branch (commit: c0947f0). That branch is based on master (1.8) which in hindsight really wasn't the best idea I've ever had because it requires you to merge it upwards for testing (shouldn't be too difficult though).

There is/are still some issue(s) with some replays (seems to mainly be on hypixel) which may or may not require further changes which is why I haven't yet pushed it into master. My goal is to have almost all replays (there are some invalid and some modded ones) uploaded to replaymod.com to parse without errors (which should be plenty of test data) before merging into master.

Afaict the most prominent issue will have to be fixed in ViaVersion itself. It seems to not be able to deal with some of the invalid entity metadata sent by hypixel (probably because it wasn't built to do so). I'm going to look further into those errors tomorrow.

Johni0702 commented 6 years ago

Above commit is now merged into all versions and part of ReplayMod *-2.1.0. Thanks for your contribution!