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

[Feature] Implement Custom Pet Names #4594

Merged
merged 7 commits into from
Jan 22, 2025

Conversation

catapultam-habeo
Copy link
Contributor

@catapultam-habeo catapultam-habeo commented Jan 10, 2025

Description

This implements functionality for renaming pets via both /changepetname and script hooks (ChangePetName() for the relevant Client classes for both lua and perl) using the vanilla EQUI_NameChangePetWnd.

Once a player has a pet name change available, they will be prompted to continue the name change process upon login if they have not yet completed it. There is a deficiency here, see below.

There is a client bug here that am unable to work around, namely that the 'Don't remind me' option does not function. The client writes this data to the player ini, but uses the wrong value when selecting this button (DisableNameChangeReminder instead of DisablePetNameChangeReminder).

There are actually three opcodes involved in this process,

  • 0x5dab which is the main data exchange, carries both name data and response codes
  • 0x4506 which invokes the pet name change window, and respects the nag-disable flag in ini (DisablePetNameChangeReminder), but this is never set correctly
  • 0x046d which immediately invokes the pet name change window, ignores the nag-disable flag, and steps on it by resetting it to 0, but also sets the wrong one.

This implementation omits the use of 0x046d in order to prevent stepping on the ability to disable nags for Player name changes, and allowing the player to manually set DisablePetNameChangeReminder if they choose to.

Fixes # (issue)

#4261

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing

(db migration)
image

(unrenamed pet)
image

(invoking window through script snippet below)

sub EVENT_SAY {
    if ($client->GetGM()) {
        if ($text eq "rename_pet") {
            $client->ChangePetName();
        }

image

(successful name change)
image

Clients tested:
Only works on ROF2

Checklist

  • I have tested my changes
  • I have performed a self-review of my code. Ensuring variables, functions and methods are named in a human-readable way, comments are added only where naming of variables, functions and methods can't give enough context.
  • I have made corresponding changes to the documentation.
  • I own the changes of my code and take responsibility for the potential issues that occur
  • If my changes make database schema changes, I have tested the changes on a local database (attach image). Updated version.h CURRENT_BINARY_DATABASE_VERSION to the new version.

@catapultam-habeo catapultam-habeo marked this pull request as ready for review January 10, 2025 22:49
@catapultam-habeo
Copy link
Contributor Author

For future reference, anyone with an MQ\2-based patching framework can fix the Disable Reminder button with the following;

    DWORD sourceAddress1 = (((DWORD)0x006fa88c - 0x400000) + baseAddress);
    DWORD destinationAddress1 = (((DWORD)0x006fa60f - 0x400000) + baseAddress);
    DWORD sourceAddress2 = (((DWORD)0x006fa88c - 0x400000) + baseAddress);
    DWORD destinationAddress2 = (((DWORD)0x006fab62 - 0x400000) + baseAddress);

    unsigned char buffer[5];

    memcpy(buffer, (void*)sourceAddress1, 5);
    PatchA((DWORD*)destinationAddress1, (char*)buffer, 5);

    memcpy(buffer, (void*)sourceAddress2, 5);
    PatchA((DWORD*)destinationAddress2, (char*)buffer, 5);
    ```

zone/pets.cpp Outdated Show resolved Hide resolved
zone/client_packet.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@Kinglykrab Kinglykrab left a comment

Choose a reason for hiding this comment

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

Looks great, the contribution is much appreciated. Minor comments, otherwise should be good to go pending 👀 from others who may want to take a look.

@catapultam-habeo
Copy link
Contributor Author

Looks great, the contribution is much appreciated. Minor comments, otherwise should be good to go pending 👀 from others who may want to take a look.

I've actually been thinking that I'm going to gate the correct packets behind a rule and PR a patch to core or something

@Kinglykrab
Copy link
Contributor

Looks great, the contribution is much appreciated. Minor comments, otherwise should be good to go pending 👀 from others who may want to take a look.

I've actually been thinking that I'm going to gate the correct packets behind a rule and PR a patch to core or something

That would be fantastic. 👍

@catapultam-habeo
Copy link
Contributor Author

I decided to just send the correct packets. The potential bug here is kind of an edge case, and the workaround is pretty bad in terms of actually mitigating the problem.

zone/client.cpp Outdated Show resolved Hide resolved
zone/perl_client.cpp Outdated Show resolved Hide resolved
zone/pets.cpp Outdated Show resolved Hide resolved
zone/pets.cpp Outdated Show resolved Hide resolved
common/database_schema.h Outdated Show resolved Hide resolved
zone/client.cpp Outdated Show resolved Hide resolved
@Akkadius
Copy link
Member

Needs rebased. Left a handful of comments.

This looks cool!

@catapultam-habeo catapultam-habeo force-pushed the rename_pet branch 2 times, most recently from 7b89cd1 to aae8f58 Compare January 22, 2025 04:12
@catapultam-habeo
Copy link
Contributor Author

Needs rebased. Left a handful of comments.

This looks cool!

The comments should now be addressed

zone/perl_client.cpp Outdated Show resolved Hide resolved
common/database_schema.h Outdated Show resolved Hide resolved
@Akkadius
Copy link
Member

Once formatting is addressed, I'm good for this one to go in

zone/client.cpp Outdated Show resolved Hide resolved
@Akkadius Akkadius merged commit 31abaf8 into EQEmu:master Jan 22, 2025
1 check passed
@Akkadius Akkadius mentioned this pull request Jan 27, 2025
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.

3 participants