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

Core/Player: Save druid/warlock customisations at barbershop #29696

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

Razinao
Copy link
Contributor

@Razinao Razinao commented Feb 14, 2024

Changes proposed:

  • Barbershop alter appearance packets come in multiple times, one for each character model (base character model, shapeshift forms, warlock pet forms, etc). At the moment, after the first packet comes in, the checks on being in a barbershop chair pass and then the character is forced to stand. Subsequent packets fail, so they don't get to perform SetCustomizations and save the changes.
  • Additionally, SetCustomizations currently performs a wipe of all customisations. We change this to only wipe the options relevant to the chrModel that is being modified. When the chrModel is 0, we fetch the customisations from the race + gender -> chrModel customisations. When this is non-zero, we fetch the chrModel customisations. In both cases we just delete any existing entries from the database/the Customisations UF.
  • ValidateAppearance was handling the customisations for a non playable race model as if it was the playable race model, so it was validating if option 911 (cat form) was available to character model Highmountain Tauren Female. This is not the case: we should be validating on if character model 190 has option 911. This most likely came from a DB2 change between 9.2.5 and now which has gone overlooked.
  • When using the barbershop or character customise, changing gender requires us to remove all of the customisations associated with the old model. This is to prevent build up of useless datapoints and keep the ValidateAppearance customisations set clean.
  • The same applies for when changing races/factions. The old race/gender combination needs to be removed entirely.
  • Additionally, when using the change race/faction for a class like druid, there are forms associated with a race. We want to remove the bear/cat forms associated with the old race.
  • Between 9.2.5 and now, a change was made so that all Bear Form options have been merged into one, and it seems like the best way to get which race was associated with the choices now is to use the RaceMask in ChrCustomizationReq. By checking through all of the old races choices and seeing if the new race also supports it, we can keep forms like the artifact / achievement forms from being reset. If the new race doesn't support it, we delete the old customisation option->choice
  • When logging in, we need to force the druid customisations to something valid as these are not pre-determined at character creation or when a race change has been completed. So, at log in, we find the first viable candidate. This will typically be a customisation that has no unlock requirements.

Issues addressed:

Closes #29429

Tests performed:

  • Verified build.

  • Verified that when using the barbershop to set druid form/warlock pet customisations, that these customisations are correctly saved.

  • Verified that when using character customisation screen to set base model customisations, that the druid form/warlock pet customisations do not get wiped.

  • Verified that when using the character changerace/changefaction, that commonly accessible customisations (i.e. warlock pets, artifact shapeshift models or other unlockables) do not get reset.

  • Verified that when using the character changerace/changefaction, that race-only accessible customisations (i.e. base bear/cat/travel forms) are forced to a valid one for the new race.

Known issues list:

  • When interactring with the barbershop in druid form or while a warlock pet is out, the customisation does not immediately take effect.

@Razinao Razinao changed the title Core/Characters: Save druid/warlock customisations at barbershop Core/Player: Save druid/warlock customisations at barbershop Feb 14, 2024
@Shauren
Copy link
Member

Shauren commented Feb 21, 2024

I do not agree with your proposed solution that introduces a dependency on customization packet order

Instead of having the main model request wipe all customizations, have each packet replace only customizations related to model being customized in that packet (this would also take care of doing it on character selection screen) - customizing dragonriding mounts also uses alter appearance packet and it doesnt send the main player model stuff at all (so you would be endlessly appending new mount customizations)

@Razinao Razinao marked this pull request as draft February 22, 2024 07:19
@Razinao Razinao marked this pull request as ready for review March 10, 2024 11:17
@KamiliaBlow
Copy link
Contributor

It looks like your changes are working a little incorrectly. You can change the appearance of your warlock pets or druid shapes. But at the same time, all other classes cannot change the appearance of their characters with the help of a hairdresser (the changes are not applied to the database). Characters of the warlock and druid class can change the appearance of a character and save it to the database.

@Aokromes
Copy link
Member

Conflicting files
src/server/game/Handlers/CharacterHandler.cpp

@Razinao
Copy link
Contributor Author

Razinao commented Apr 1, 2024

It looks like your changes are working a little incorrectly. You can change the appearance of your warlock pets or druid shapes. But at the same time, all other classes cannot change the appearance of their characters with the help of a hairdresser (the changes are not applied to the database). Characters of the warlock and druid class can change the appearance of a character and save it to the database.

Thanks for catching this, this is fixed with the latest commit.

@thorton1786
Copy link

Hello sorry to be a pain I’m greatful for your work and it seems to be working great. The only issue I seem to be having still is the barbershop changes resetting for worgen druids. I haven’t had the issue with any other races as of yet.

@Razinao
Copy link
Contributor Author

Razinao commented May 13, 2024

Hello sorry to be a pain I’m greatful for your work and it seems to be working great. The only issue I seem to be having still is the barbershop changes resetting for worgen druids. I haven’t had the issue with any other races as of yet.

This was being caused because the human customizations were not being removed, and so the transaction to commit new customizations was inserting partially duplicated results.

I was misusing the chrModel for this scenario when I should've been getting customisations from race+gender which contains the parent model customisations.

Shoulds be fixed by the two most recent commits.

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

Successfully merging this pull request may close these issues.

Moonkin/Druid Customizations Don't Save
5 participants