-
Notifications
You must be signed in to change notification settings - Fork 84
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 an event to fill animations for player #870
base: dev
Are you sure you want to change the base?
Conversation
/// <br/> <br/> | ||
/// Mods using this should always add an id to <see cref="List{T}"/> ids whether what the <see cref="string"/> id was given, to avoid certain <see cref="string"/> id missing their animations | ||
/// </summary> | ||
public static event GetIdsUsedFillAnimForIdHandler OnGetIdsUsedFillAnimFor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't like this name; please make it more descriptive. it should be "OnSomeEvent" where SomeEvent is the description of what event happened
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not good at naming, what do you think of GetIdsToFindAndFillAnimFor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or FindIdsToFillAnimOfTheirFor
I messed up the merge :despair: it's nearly impossible to resolve online because you "modified" the whole file (probably has to do with line endings) |
13ad2a4
to
bf5d29b
Compare
I force-pushed to cancel the merge, and just converted Everest.Events to Linux line endings locally, that did it for resolving the "conflict" |
No description provided.