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

Legacy Packet Adapter #59

Merged
merged 25 commits into from
Oct 13, 2024
Merged

Legacy Packet Adapter #59

merged 25 commits into from
Oct 13, 2024

Conversation

Nixuge
Copy link
Contributor

@Nixuge Nixuge commented Aug 24, 2024

Very rough edges but had free time so why not

Confirmed to work on 1.7.10 -> 1.12.2 instead of just 1.8.8

TODOs:

  • Change the name of RandomUtils
  • Actually test other versions (urgent), especially 1.7 & 1.12.2 as those are pretty much the main point of this PR
  • Cache more reflections things (See RandomUtils#22)
  • Test one thing that I couldn't manage to trigger (See TeamsPacketAdapterImpl.java#97, first part also in the tests for 1.8.8 only and passing)
  • Remove the debug print for last point
  • (idk?) add a dash in the middle of the packet adapter name (legacy-reflections instead of legacyreflections? Named it like that bc you can't have a - in java classes)
    & probably some other things I forgot
  • add collision rule

Maybe TODO:

  • Yoink out 1.8.8 packet adapter? This packet adapter is basically the exact same one as the 1.8.8 one but w reflections in it. (Would break people's build.gradle, altho easily fixable)

@vytskalt
Copy link
Collaborator

I think we remove the 1.8 adapter and rename this one to just legacy since I don't see a reason for 1.8 to exist anymore (performance diff should be minimal and it doesn't matter anyway since it's all done async). For backwards compat the v1_8_R3 module could stay and would just do something like implementation(project(":legacy")) in the build.gradle.

@vytskalt vytskalt self-assigned this Aug 25, 2024
@vytskalt vytskalt added the enhancement New feature or request label Aug 25, 2024
@Nixuge
Copy link
Contributor Author

Nixuge commented Aug 25, 2024

Update:
Tested 3 versions:

  • 1.8.8 (v1_8_R3)
  • 1.7.10 (v1_7_R4)
  • 1.12.2 (v1_12_R1)

Out of those three:

  • 1.8.8 works just fine (tested with a "somewhat" complex setup, still requires more testing to make sure all of the code is working)
  • 1.12.2 seems to work fine with the simple example (needs a lot more testing)
  • 1.7.10 unfortunately doesn't work, the following were missing in that version:
    • enumScoreboardActionClass
    • enumScoreboardHealthDisplayClass
    • enumScoreboardHealthDisplayClass

Removing those and trying to build unfortunately doesn't seem to make it work either.

Would guess this works fine for all versions from 1.8 to 1.12.2. Unfortunately not 1.7, may make another PR for those versions. Still begs the question as to if this should be called "legacy" or "1.8-1.12", since if 1.7 requires another adapter that one would need to be named legacy-legacier or smth (depends on if you even want a 1.7- adapter at all in this project)

Also completely forgot on the initial comment, but another urgent thing is to get the "v1_8_R3" , "v1_12_R1" from the server version as of rn it's just hardcoded in RandomUtils

@vytskalt
Copy link
Collaborator

FastBoard supports 1.7.10, we can use it as a reference as to what needs changing. https://github.com/MrMicky-FR/FastBoard/blob/master/src/main/java/fr/mrmicky/fastboard/FastBoardBase.java

@Nixuge
Copy link
Contributor Author

Nixuge commented Aug 25, 2024

From a quick glance, most likely place that's failing in 1.7 with nothing changed is in sendScore:

  @Override
  public void sendScore(
...
  ) {
    Object packet = packetPlayOutScoreboardScoreConstructor.invoke(entry);
    PacketAccessors.SCORE_OBJECTIVE_NAME_FIELD.set(packet, objectiveName);
    PacketAccessors.SCORE_VALUE_FIELD.set(packet, value);
//    Removed - not in 1.7
//    PacketAccessors.SCORE_ACTION_FIELD.set(packet, enumScoreboardActionChange);
    sender.sendPacket(players, packet);
  }

In FastBoard, they still do something for the action field even in 1.7:

        if (VersionType.V1_8.isHigherOrEqual()) {
            Object enumAction = action == ScoreboardAction.REMOVE
                    ? ENUM_SB_ACTION_REMOVE : ENUM_SB_ACTION_CHANGE;
            setField(packet, ENUM_SB_ACTION, enumAction);
        } else {
            setField(packet, int.class, action.ordinal(), 1); // Action
        }

@Nixuge
Copy link
Contributor Author

Nixuge commented Aug 25, 2024

image
And indeed that was the fix

Added this in PacketAccessors:

  public static final FieldAccessor<Object, Integer> SCORE_ACTION_FIELD_1_7 =
    ReflectUtil.findField(packetPlayOutScoreboardScoreClass, 1, int.class);

Then in ObjectivePacketAdapterImpl replaced:

PacketAccessors.SCORE_ACTION_FIELD.set(packet, enumScoreboardActionChange);

with

PacketAccessors.SCORE_ACTION_FIELD_1_7.set(packet, 0); // 0 = CHANGE, int value yoinked from FastBoard

Pretty sure the other lib's functionalities that i had to remove for 1.7 support were added in 1.8 tho so can't support them

@Nixuge
Copy link
Contributor Author

Nixuge commented Aug 25, 2024

Unfortunately, as excepted, 1.6 doesn't work either with that last change.

@Nixuge
Copy link
Contributor Author

Nixuge commented Aug 27, 2024

Not sure why the test is failing btw, iirc it worked correctly when I tried it

@vytskalt vytskalt changed the title Added Legacy Reflections Packet Adapter Legacy Packet Adapter Oct 8, 2024
@vytskalt vytskalt merged commit 97cd00b into MegavexNetwork:main Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants