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

item: Implement CoinCollectDummy #221

Merged
merged 2 commits into from
Jan 4, 2025

Conversation

german77
Copy link
Contributor

@german77 german77 commented Jan 3, 2025

This is used in HelpAmiiboCoinCollect. I'm stuck in that one so for now I will implement all coin collect variants.


This change is Reviewable

@german77 german77 force-pushed the coinCollectDummy branch 4 times, most recently from eff3f65 to bd9b269 Compare January 3, 2025 01:20
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 r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @german77)


src/Item/CoinCollectDummy.h line 11 at r1 (raw file):

class CoinCollectDummy : public al::LiveActor {
public:
    CoinCollectDummy(const char*);

Suggestion:

CoinCollectDummy(const char* name);

src/Item/CoinCollectDummy.h line 13 at r1 (raw file):

    CoinCollectDummy(const char*);

    void init(const al::ActorInitInfo&) override;

Suggestion:

void init(const al::ActorInitInfo& initInfo) override;

src/Item/CoinCollectDummy.h line 16 at r1 (raw file):

    void appear() override;

    void appearHint(const sead::Vector3f&);

Suggestion:

void appearHint(const sead::Vector3f& position);

src/Item/CoinCollectHintState.h line 12 at r1 (raw file):

public:
    CoinCollectHintState(al::LiveActor*);
    ~CoinCollectHintState();

Remove this - it appears to exist, but that's only because it's auto-generated by the compiler. Not specifying it here is the right way to go.
(also, it would've required override otherwise)


src/Item/CoinCollectDummy.cpp line 19 at r1 (raw file):

struct {
    NERVE_MAKE(CoinCollectDummy, Hint);
} NrvCoinCollectDummy;

Suggestion:

NERVES_MAKE_STRUCT(CoinCollectDummy, Hint);

Copy link
Contributor Author

@german77 german77 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: 1 of 4 files reviewed, 5 unresolved discussions (waiting on @MonsterDruide1)


src/Item/CoinCollectHintState.h line 12 at r1 (raw file):

Previously, MonsterDruide1 wrote…

Remove this - it appears to exist, but that's only because it's auto-generated by the compiler. Not specifying it here is the right way to go.
(also, it would've required override otherwise)

Done.


src/Item/CoinCollectDummy.cpp line 19 at r1 (raw file):

struct {
    NERVE_MAKE(CoinCollectDummy, Hint);
} NrvCoinCollectDummy;

Done.


src/Item/CoinCollectDummy.h line 11 at r1 (raw file):

class CoinCollectDummy : public al::LiveActor {
public:
    CoinCollectDummy(const char*);

Done.


src/Item/CoinCollectDummy.h line 13 at r1 (raw file):

    CoinCollectDummy(const char*);

    void init(const al::ActorInitInfo&) override;

Done.


src/Item/CoinCollectDummy.h line 16 at r1 (raw file):

    void appear() override;

    void appearHint(const sead::Vector3f&);

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 r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @german77)

@MonsterDruide1 MonsterDruide1 merged commit c3725f6 into MonsterDruide1:master Jan 4, 2025
6 checks passed
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