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

Barber #448

Merged
merged 5 commits into from
Jul 7, 2017
Merged

Barber #448

merged 5 commits into from
Jul 7, 2017

Conversation

OptimShi
Copy link
Collaborator

@OptimShi OptimShi commented Jul 4, 2017

Added "@barbershop" debug command and corresponding functionality to load the interface, and save and apply settings.
Changed some CharGen types that were ushort to uints. (They were short for legacy reasons no longer needed)

Not sure if the HandleActionFinishBarber() function is the "correct" method, so please feel free to point out what I may not be doing the "proper way".

@fartwhif
Copy link
Collaborator

fartwhif commented Jul 4, 2017

tried it out, very cool

Copy link
Contributor

@ddevec ddevec left a comment

Choose a reason for hiding this comment

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

Just a couple of concurrency issues. Should be simple fixes.

{
// Read the payload sent from the client...
PaletteGuid = message.Payload.ReadUInt32();
Character.HeadObject = message.Payload.ReadUInt32();
Copy link
Contributor

Choose a reason for hiding this comment

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

These operations modify the Player (through the character). Therefore they need to be run as part of the Player's Action's. Do this through an ActionChain (just put the entire thing inside an ActionChain).

const uint EmpyreanFemaleMotionDID = 0x0900020Du;

// We will use this function to get the current player's appearance values.
AceCharacter characterClone = (AceCharacter)Session.Player.GetAceObject();
Copy link
Contributor

Choose a reason for hiding this comment

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

This accesses the player. If the player is modified while its being cloned it may crash the server. Put this inside an ActionChain to mitigate that issue.

@OptimShi
Copy link
Collaborator Author

OptimShi commented Jul 6, 2017

I moved the Barber functions into an ActionChain, as requested.

@@ -13,7 +7,9 @@ public static class GameActionFinishBarber
[GameAction(GameActionType.FinishBarber)]
public static void Handle(ClientMessage message, Session session)
{
session.Player.HandleActionFinishBarber(message);
ActionChain chain = new ActionChain();
chain.AddAction(session.Player, () => session.Player.HandleActionFinishBarber(message));
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer that the action chain is within HandleActionFInishBarber (HandleAction indicates that the function is safe to call from outside of the player's action context). I wont block the PR over it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will adjust this, then. Because no one, including myself, will ever go back and do it later! Hold off on pushing, please.

@ddevec ddevec merged commit 2d60fd8 into ACEmulator:master Jul 7, 2017
@OptimShi OptimShi deleted the Barber branch July 10, 2017 16:43
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.

None yet

4 participants