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

Actor: BarrierField #90

Merged
merged 8 commits into from
Jun 16, 2024

Conversation

LynxDev2
Copy link
Contributor

@LynxDev2 LynxDev2 commented Jun 14, 2024

Implements all functions for the barrier actor used in boss battles with The Broodals


This change is Reviewable

Copy link
Owner

@MonsterDruide1 MonsterDruide1 left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @LynxDev2)


lib/al/Library/Nerve/NerveSetupUtil.h line 36 at r1 (raw file):

        }                                                                                          \
                                                                                                   \
        const char* getActionName() const { return #Action; }                                      \

Might cause the four getActionName functions to match? (will be inlined otherwise)

Suggestion:

virtual const char* getActionName() const { return #Action; }

src/MapObj/BarrierField.h line 25 at r1 (raw file):

private:
    bool mIsDisappearByShineGet = false;
    bool mIsOnTheMoon = false;

Shorter name to align with other, similar names throughout the game

Suggestion:

bool mIsMoon = false;

src/MapObj/BarrierField.cpp line 30 at r1 (raw file):

    NERVE_MAKE(BarrierField, Hide);
    NERVE_MAKE(BarrierField, Disappear);
    void* padding[4];

sub_710000A098 explains this gap. The entire function seems like it could be another macro, not sure about it yet though. This function is referenced in the .init_array section, so it is some kind of initializer that is called before the program even starts.


src/MapObj/BarrierField.cpp line 45 at r1 (raw file):

    if (al::isObjectName(initInfo, "WaterfallWorldHomeBarrier")) {
        GameDataHolderAccessor dataAccessor(this);
        if (GameDataFunction::isWorldMoon(dataAccessor))

Suggestion:

        if (GameDataFunction::isWorldMoon(this))

src/Util/StageSceneFunction.h line 12 at r1 (raw file):

class IJudge;
class BarrierField;

Useless?

Copy link
Contributor Author

@LynxDev2 LynxDev2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 7 files reviewed, 5 unresolved discussions (waiting on @MonsterDruide1)


lib/al/Library/Nerve/NerveSetupUtil.h line 36 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Might cause the four getActionName functions to match? (will be inlined otherwise)

Done.


src/MapObj/BarrierField.h line 25 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Shorter name to align with other, similar names throughout the game

Done.


src/MapObj/BarrierField.cpp line 30 at r1 (raw file):

Previously, MonsterDruide1 wrote…

sub_710000A098 explains this gap. The entire function seems like it could be another macro, not sure about it yet though. This function is referenced in the .init_array section, so it is some kind of initializer that is called before the program even starts.

interesting


src/MapObj/BarrierField.cpp line 45 at r1 (raw file):

    if (al::isObjectName(initInfo, "WaterfallWorldHomeBarrier")) {
        GameDataHolderAccessor dataAccessor(this);
        if (GameDataFunction::isWorldMoon(dataAccessor))

Done.


src/Util/StageSceneFunction.h line 12 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Useless?

Done.

Copy link
Owner

@MonsterDruide1 MonsterDruide1 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 5 files at r2, all commit messages.
Reviewable status: 5 of 7 files reviewed, 2 unresolved discussions (waiting on @LynxDev2)


lib/al/Library/Nerve/NerveSetupUtil.h line 36 at r1 (raw file):

Previously, LynxDev2 wrote…

Done.

Actually, it's something entirely different. Blocked by #91, then this can be rebased and use the new macros.

@LynxDev2 LynxDev2 force-pushed the BarrierFieldActor branch from 93cd105 to f22fa7d Compare June 16, 2024 13:14
Copy link
Contributor Author

@LynxDev2 LynxDev2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 7 files reviewed, 2 unresolved discussions (waiting on @MonsterDruide1)


lib/al/Library/Nerve/NerveSetupUtil.h line 36 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Actually, it's something entirely different. Blocked by #91, then this can be rebased and use the new macros.

Done.

Copy link
Owner

@MonsterDruide1 MonsterDruide1 left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @LynxDev2)


data/odyssey_functions.csv line 0 at r4 (raw file):
Assign a name to 710000a098 according to tools/listsym to get an additional match

Copy link
Owner

@MonsterDruide1 MonsterDruide1 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @LynxDev2)


src/MapObj/BarrierField.h line 6 at r4 (raw file):

namespace al {
class LiveActor;

Useless forward-declare (already included + has to be included due to inheritance)


src/MapObj/BarrierField.cpp line 17 at r4 (raw file):

namespace alNerveFunction {
class NerveActionCollector;
}

Useless forward-declare

Code quote:

namespace alNerveFunction {
class NerveActionCollector;
}

src/MapObj/BarrierField.cpp line 38 at r4 (raw file):

        if (GameDataFunction::isWorldMoon(this))
            mIsMoon = true;
    }

Suggestion:

    if (al::isObjectName(initInfo, "WaterfallWorldHomeBarrier") && GameDataFunction::isWorldMoon(this))
            mIsMoon = true;

Copy link
Contributor Author

@LynxDev2 LynxDev2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @MonsterDruide1)


data/odyssey_functions.csv line at r4 (raw file):

Previously, MonsterDruide1 wrote…

Assign a name to 710000a098 according to tools/listsym to get an additional match

Done.


src/MapObj/BarrierField.h line 6 at r4 (raw file):

Previously, MonsterDruide1 wrote…

Useless forward-declare (already included + has to be included due to inheritance)

Done.


src/MapObj/BarrierField.cpp line 17 at r4 (raw file):

Previously, MonsterDruide1 wrote…

Useless forward-declare

Done.


src/MapObj/BarrierField.cpp line 38 at r4 (raw file):

        if (GameDataFunction::isWorldMoon(this))
            mIsMoon = true;
    }

Done.

Copy link
Owner

@MonsterDruide1 MonsterDruide1 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @LynxDev2)

@MonsterDruide1 MonsterDruide1 merged commit 49c372a into MonsterDruide1:master Jun 16, 2024
5 checks passed
@LynxDev2 LynxDev2 deleted the BarrierFieldActor branch September 29, 2024 16:06
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.

2 participants