Skip to content

Conversation

@leggettc18
Copy link
Contributor

@leggettc18 leggettc18 commented Mar 10, 2023

Refactors to allow the above two arrays to be a dynamic size when the game launches, size is set during the AudioLoad_Init function. I have already confirmed that AudioLoad_Init only gets called once per launch, and on soft-reset, AudioLoad_Init does not get called again, so these memory allocations do not cause any memory leaks.

Build Artifacts

Refactors to allow the above two arrays to be a dynamic size when the game launches, size is set during the AudioLoad_Init function.
@leggettc18 leggettc18 requested a review from briaguya0 March 10, 2023 15:18
@leggettc18
Copy link
Contributor Author

leggettc18 commented Mar 10, 2023

Would we want this in Khan Bravo or is this too far removed from being a bugfix for that?

@leggettc18 leggettc18 requested a review from dcvz March 10, 2023 16:00
@briaguya0
Copy link
Contributor

I could see this being a Khan Bravo thing (assuming testing goes well)

@leggettc18
Copy link
Contributor Author

OK I'll leave this up for people to test with and backport the commit if that goes well.

@jbodner09
Copy link
Contributor

Just because we've had problems with it in the past, does the init also not get called again when fast file select is turned on?

@Malkierian
Copy link
Contributor

No, AudioInit is called well before the title screen, and isn't called at any point after that, fast file select or no.

@leggettc18
Copy link
Contributor Author

I did not test with fast file select turned on. Considering regular music works fine with fast file select it should still be fine with fast file select but I will confirm later.

@Malkierian
Copy link
Contributor

I did confirm. The results were as I said.

@leggettc18
Copy link
Contributor Author

I've been playing through a rando run on this with no issues (and before introducing this I did have crashing issues due to too many custom sequences) so if no one else has run into any issues I think we can call this good. Now the question is do we want to merge this directly to dev or to Khan first via #2610?

@briaguya0
Copy link
Contributor

I'm leaning khan for it. Considering it's been well tested and prevents issues with too many sequences it feels close enough to a bugfix to get in khan

@leggettc18
Copy link
Contributor Author

Closing because #2610 was merged. A future khan->dev merge will bring this functionality into the main develop branch.

@leggettc18 leggettc18 closed this Mar 12, 2023
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.

4 participants