Skip to content

Audio: Implement CollectBgmPlayer::prepare, SePlayObjWithSave::init, and ShopBgmPlayer::movement - #1283

Open
DrapNard wants to merge 1 commit into
MonsterDruide1:masterfrom
DrapNard:audio/lifecycle-audio-methods
Open

Audio: Implement CollectBgmPlayer::prepare, SePlayObjWithSave::init, and ShopBgmPlayer::movement#1283
DrapNard wants to merge 1 commit into
MonsterDruide1:masterfrom
DrapNard:audio/lifecycle-audio-methods

Conversation

@DrapNard

@DrapNard DrapNard commented Jun 21, 2026

Copy link
Copy Markdown

Implements the following matching audio functions:

  • CollectBgmPlayer::prepare()
  • SePlayObjWithSave::init(...)
  • ShopBgmPlayer::movement()

Validation:

  • python tools/build.py ✅
  • tools/check ✅
  • individual tools/check -mw for each function ✅
  • git diff --check ✅

No unrelated files were modified.


This change is Reviewable


Report for 1.0 (8d9f865 - 30709b1)

📈 Matched code: 15.18% (+0.00%, +12 bytes)

✅ 3 new matches
Unit Item Bytes Before After
Audio/CollectBgmPlayer CollectBgmPlayer::prepare() +4 0.00% 100.00%
Audio/SePlayObjWithSave SePlayObjWithSave::init(al::ActorInitInfo const&) +4 0.00% 100.00%
Audio/ShopBgmPlayer ShopBgmPlayer::movement() +4 0.00% 100.00%

Copilot AI review requested due to automatic review settings June 21, 2026 22:16
@github-actions github-actions Bot added the status:waiting for review This PR is ready to take the first or another look at it! label Jun 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR completes three small audio-related functions by adding minimal class definitions and implementations that match the original binary’s behavior, and updates the decompilation status entries accordingly.

Changes:

  • Added ShopBgmPlayer as an al::LiveActor and implemented ShopBgmPlayer::movement() as a base-call passthrough.
  • Added SePlayObjWithSave and implemented SePlayObjWithSave::init(...) as a base-call passthrough.
  • Added CollectBgmPlayer and implemented CollectBgmPlayer::prepare() as an empty function (return-only), then marked all three symbols as Matching in data/file_list.yml.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Audio/ShopBgmPlayer.h Declares ShopBgmPlayer actor type and its movement() override.
src/Audio/ShopBgmPlayer.cpp Implements movement() as al::LiveActor::movement() passthrough for matching.
src/Audio/SePlayObjWithSave.h Declares SePlayObjWithSave and minimal al::SePlayObj dependency needed for init(...).
src/Audio/SePlayObjWithSave.cpp Implements init(...) as al::SePlayObj::init(info) passthrough for matching.
src/Audio/CollectBgmPlayer.h Declares CollectBgmPlayer::prepare().
src/Audio/CollectBgmPlayer.cpp Implements prepare() as an empty function for matching.
data/file_list.yml Updates symbol statuses to Matching for the three implemented functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@german77 german77 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Welcome to the project. Weird that there's an AI response here.

@german77 reviewed 7 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on DrapNard).


src/Audio/CollectBgmPlayer.h line 6 at r1 (raw file):

public:
    void prepare();
};

When adding new headers please include all class functions. This avoids some work in the future and is helpful for mod creation. You can use as reference MonsterDruide1/OdysseyDecompTracker#20

Suggestion:

class CollectBgmPlayer : public al::HioNode, public al::ISceneObj {
public:
    CollectBgmPlayer();
    void init(const al::IUseAudioKeeper*);
    void prepare();
    void start(const char*, const char*);
    void stop(s32);
    void isPlaying(const char*, const char*) const;

private:
    char filler[0x18];
};

static_assert(sizeof(CollectBgmPlayer) == 0x20);

src/Audio/SePlayObjWithSave.h line 6 at r1 (raw file):

class ActorInitInfo;

class SePlayObj {

SePlayObj goes into Library/Obj/SePlayObj.h add the remaining functions of this class as well MonsterDruide1/OdysseyDecompTracker#1905

Suggestion:

class SePlayObj : public al::LiveActor {
....
private:
    char filler[0x98];
};

static_assert(sizeof(SePlayObj) == 0x1a0);

src/Audio/SePlayObjWithSave.h line 15 at r1 (raw file):

public:
    void init(const al::ActorInitInfo& info);
};

Add the remaining functions of this class as well MonsterDruide1/OdysseyDecompTracker#24

Suggestion:

class SePlayObjWithSave : public al::SePlayObj {
public:
    void init(const al::ActorInitInfo& info);
};
static_assert(sizeof(SePlayObjWithSave) == 0x1a0);

src/Audio/ShopBgmPlayer.h line 10 at r1 (raw file):

    void movement() override;
};

using al::LiveActor::LiveActor; is not required
Add the remaining functions of this class as well MonsterDruide1/OdysseyDecompTracker#25

Suggestion:

class ShopBgmPlayer : public al::LiveActor {
public:
    void movement() override;
private:
    char filler[0x10];
};
static_assert(sizeof(SePlayObjWithSave) == 0x118);

@github-actions github-actions Bot added status:waiting for author Reviews have shown that the author of this PR needs to change some things. and removed status:waiting for review This PR is ready to take the first or another look at it! labels Jun 21, 2026
@DrapNard

Copy link
Copy Markdown
Author

Thanks ! The AI response is because I have copilot education :3

And I use a lot of AI in this PR so I think you can add the label AI to the PR.

Also thanks for the suggestion I will work later on it !

@DrapNard
DrapNard force-pushed the audio/lifecycle-audio-methods branch from c6435bb to 30709b1 Compare June 22, 2026 08:51
@MonsterDruide1 MonsterDruide1 added the ai This PR has been created with the help of AI tools and is handled with a lower priority. label Jun 22, 2026

@german77 german77 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@german77 reviewed 3 files, made 1 comment, and resolved 4 discussions.
Reviewable status: 7 of 8 files reviewed, 1 unresolved discussion (waiting on DrapNard).


src/Audio/CollectBgmPlayer.h line 19 at r2 (raw file):

    void start(const char* name, const char* situation_name);
    void stop(s32 fade_frames);
    void isPlaying(const char* name, const char* situation_name) const;

How do you arrived to these parameter names?

Suggestion:

    bool isPlaying(const char* name, const char* situation_name) const;

@github-actions github-actions Bot added status:waiting for author Reviews have shown that the author of this PR needs to change some things. and removed status:waiting for author Reviews have shown that the author of this PR needs to change some things. labels Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai This PR has been created with the help of AI tools and is handled with a lower priority. status:waiting for author Reviews have shown that the author of this PR needs to change some things.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants