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

Add OnActorKill & OnEnemyDefeat hooks #3112

Merged
merged 6 commits into from Sep 10, 2023

Conversation

Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Aug 9, 2023

Adds the OnActorKill & OnEnemyDefeat hooks.
Also moves the "Enemies Defeated" stats to mods.cpp

Build Artifacts

@@ -87,7 +87,7 @@ typedef enum {
COUNT_ENEMIES_DEFEATED_PARASITIC_TENTACLE, // EN_BA
COUNT_ENEMIES_DEFEATED_PEAHAT, // EN_PEEHAT
COUNT_ENEMIES_DEFEATED_PEAHAT_LARVA, // EN_PEEHAT
COUNT_ENEMIES_DEFEATED_POE, // EN_POH
COUNT_ENEMIES_DEFEATED_POE, // EN_POH & EN_PO_FIELD
Copy link
Contributor

Choose a reason for hiding this comment

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

is this bringing things in line with how it was actually working before or changing the meaning?

was it

COUNT_ENEMIES_DEFEATED_POE means "non-big poes defeated" and COUNT_ENEMIES_DEFEATED_POE_BIG means "big poes defeated" before, and this is changing it to mean COUNT_ENEMIES_DEFEATED_POE meaning "big and non-big poes defeated" and COUNT_ENEMIES_DEFEATED_POE_BIG meaning "big poes defeated"

or was it always

COUNT_ENEMIES_DEFEATED_POE meaning "big and non-big poes defeated" and COUNT_ENEMIES_DEFEATED_POE_BIG meaning "big poes defeated"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

COUNT_ENEMIES_DEFEATED_POE means "non-big poes defeated" but ACTOR_EN_PO_FIELD is used by every poe in hyrule field, even the small ones, I just updated the comment to reflect that.

Here's the code for counting the kills:

case ACTOR_EN_POH:
    if (actor->params == EN_POH_FLAT || actor->params == EN_POH_SHARP) {
        gSaveContext.sohStats.count[COUNT_ENEMIES_DEFEATED_POE_COMPOSER]++;
    } else {
        gSaveContext.sohStats.count[COUNT_ENEMIES_DEFEATED_POE]++;
    }
    break;

case ACTOR_EN_PO_FIELD:
    if (actor->params == EN_PO_FIELD_BIG) {
        gSaveContext.sohStats.count[COUNT_ENEMIES_DEFEATED_POE_BIG]++;
    } else {
        gSaveContext.sohStats.count[COUNT_ENEMIES_DEFEATED_POE]++;
    }
    break;

@garrettjoecox
Copy link
Contributor

@Pepe20129 not sure if you saw but the builds here failed

@Pepe20129
Copy link
Contributor Author

The only change in the commit that has broken builds is the removal of comments so it shouldn't have failed. I have made an empty commit to re-run the build.

@Pepe20129
Copy link
Contributor Author

It turns out that the first commit didn't run any builds even though it displays a checkmark.
The problem was that in non-windows platforms the missing include statements were causing the build to fail.

Copy link
Contributor

@garrettjoecox garrettjoecox left a comment

Choose a reason for hiding this comment

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

Looks like you need to resolve a conflict but otherwise LGTM

@Pepe20129
Copy link
Contributor Author

Fixed the confict.

@garrettjoecox garrettjoecox merged commit 5cf0eee into HarbourMasters:develop Sep 10, 2023
8 checks passed
@Pepe20129 Pepe20129 deleted the more_hooks branch September 10, 2023 17:31
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.

None yet

3 participants