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

Add Threat API Packet Handling #44

Closed
wants to merge 1 commit into from

Conversation

insunaa
Copy link
Contributor

@insunaa insunaa commented May 28, 2022

This PR adds handlers for Threat API Opcodes.
They are currently unimplemented on CMaNGOS, not sure about the status on VMaNGOS but I believe also not implemented; but someone may decide to add the API to their implementation of the server, hence this.

This PR does explicitly not define the values for these Opcodes, as I don't want to define values for them, without making sure that these values don't collide with other Opcode-Values or cause any other issue.

I'll leave the PR in draft, and request comments/suggestions on how to proceed. (I am not entirely sure which data-type the Threat values are supposed to be. The TrinityCore project seems to believe they are 64 bit signed integers ( https://github.com/TrinityCore/TrinityCore/blob/wotlk_classic/src/server/game/Server/Packets/CombatPackets.h#L94 ), but I couldn't independently verify this.)

@0blu
Copy link
Collaborator

0blu commented May 28, 2022

I also had an idea like this "Add support for forward compatible cores" to add support for the "get-early-off-taxi-button" and modern warden. It can be enabled via config and forwards new opcodes to the server / client. (Maybe with an opcode prefix to prevent collisions with old ones).

@insunaa
Copy link
Contributor Author

insunaa commented May 29, 2022

The idea I had for gating these Opcodes serverside was to only send the new Opcodes to clients that have the OS identifier set to "HRM" or something like that, instead of "Win" or "OSX". That way the server stays compatible with the old clients, while enabling the "enhanced" functionality for new clients.

@0blu
Copy link
Collaborator

0blu commented May 29, 2022

We have 4 bytes(uint32) for the OS/Platform identifier this would fit the modern identifier.
If ForwardCompatibleCore is set to true in the config the ClientBuild, ReportedOS and ReportedPlatform are ignored.
HermesProxy will send the ModernValues so that the Core knows which WardenModules to send.
Mangos would receive these values Platform/OS: Wn64, Build: 40618.

@insunaa
Copy link
Contributor Author

insunaa commented May 29, 2022

Wouldn't that require the core to itself implement all the new opcodes already? At that point why use Hermes? I think of this more as an extension, where the server is allowed to send specific opcodes outside of the defined original opcodes that Hermes then translates / passes along. There could be some extra opcode in the auth process if the client identifies as "HRM" that negotiates a list of common opcodes? As in the server sends a list: "Here are the extended opcodes I can receive and handle and the extended opcodes that I can send"

@0blu
Copy link
Collaborator

0blu commented May 29, 2022

All opcodes stay the same, new ones like ThreadAPI, GetOffTaxiButton, Warden, will be prefixed by Mangos so that HermesProxy knows that they can be forwarded. The prefix can be something like 0xFFFFFFFE and the next value will be the UniversalOpcode.

@insunaa
Copy link
Contributor Author

insunaa commented May 29, 2022

https://github.com/insunaa/HermesProxy/tree/threat_testimpl

https://github.com/insunaa/mangos-tbc/tree/hermesenhanced

For testing I created an actual implementation with CMaNGOS TBC, this is how it looks and works.

@ratkosrb
Copy link
Collaborator

It seems too ghetto to have custom opcodes and packets in hermes, unless this is an official part of cmangos.

@insunaa
Copy link
Contributor Author

insunaa commented May 29, 2022

In my test I just used the WotLK opcodes because the API exists there.
And the rest of the threat API is a backport of WotLK code

As for custom opcodes I propose only a single custom opcode, which is an Opcode that negotiates a list of Opcodes that the Server can handle/send, out of the list of opcodes we have defined in the 1_14 / 2_5_2 enums, not entirely custom opcodes

The point here is to extend the capability of "standard" cmangos/vmangos, without the expressed intent to have them become part of official cmangos/official vmangos, unless maybe in a separate development branch or a fork

@ratkosrb
Copy link
Collaborator

I'm okay with implementing wotlk opcodes, we could support wotlk classic too at some point. But don't add in non-existent opcodes in the 1.12 or 2.4.3 opcodes enums, unless these opcodes are added to mainline cmangos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants