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

Fix some wrong return values in mob skill code #2887

Merged
merged 6 commits into from
Nov 16, 2020

Conversation

Kenpachi2k13
Copy link
Member

Pull Request Prelude

Changes Proposed

  • Fix return value evaluation of mob_skill_use() in mob_ai_sub_hard().
  • Fix nullpo-check return values in mob_skill_use().
  • Fix nullpo-check return values and initial value of variable ret in mobskill_event().
  • Rename mob->skill_use() to mob->use_skill() due to changed return values.
  • Rename mob->skill_event() to mob->use_skill_event() due to changed return values.

Issues addressed: None.

@Kenpachi2k13 Kenpachi2k13 added type:bug Issue is a bug or describes an incorrect behavior that should be fixed component:core Affecting the Hercules core (i.e. not the game mechanics directly) status:code-review Awaiting code review hacktoberfest-accepted Easy-to-tackle issues labels Oct 31, 2020
@Kenpachi2k13 Kenpachi2k13 added this to the Release v2020.11.16 milestone Oct 31, 2020
@Kenpachi2k13 Kenpachi2k13 self-assigned this Oct 31, 2020
@skyleo
Copy link
Contributor

skyleo commented Nov 5, 2020

What confuses me a bit about this PR is that I see no commit that is purely dedicated to changing the return value logic.
But there's commits that rename functions due to their return value logic having changed. Can you post a reference to the previous commits that I need to know for this?

@skyleo
Copy link
Contributor

skyleo commented Nov 5, 2020

mob_use_skill has a typo in it's documentation btw.

@Kenpachi2k13
Copy link
Member Author

What confuses me a bit about this PR is that I see no commit that is purely dedicated to changing the return value logic.
But there's commits that rename functions due to their return value logic having changed. Can you post a reference to the previous commits that I need to know for this?

Aye-aye, sir!
452b603

mob_use_skill has a typo in it's documentation btw.

Captain eagle eye. 😆
I'll fix it....

@skyleo
Copy link
Contributor

skyleo commented Nov 5, 2020

@Kenpachi2k13 I think you have to set res = 1 in mobskill_event as well due to how the return values changed.
I see you did that here already. hehe

@Kenpachi2k13
Copy link
Member Author

@Kenpachi2k13 I think you have to set res = 1 in mobskill_event as well due to how the return values changed.

I did. 😮
521ca00

But thanks to your comment I just realised that I forgot to separate the declaractions of target_id and res. 👍

@MishimaHaruna MishimaHaruna merged commit 608e600 into HerculesWS:master Nov 16, 2020
@Kenpachi2k13 Kenpachi2k13 deleted the mob_skill_code branch November 16, 2020 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:core Affecting the Hercules core (i.e. not the game mechanics directly) hacktoberfest-accepted Easy-to-tackle issues status:code-review Awaiting code review type:bug Issue is a bug or describes an incorrect behavior that should be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants