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 Sound System to GTCEu #234

Merged
merged 29 commits into from
Nov 26, 2021
Merged

Add Sound System to GTCEu #234

merged 29 commits into from
Nov 26, 2021

Conversation

bruberu
Copy link
Member

@bruberu bruberu commented Nov 12, 2021

Adds sounds to the specifications of issue #221.

@bruberu bruberu force-pushed the sounds branch 3 times, most recently from 2902616 to aecdec0 Compare November 20, 2021 17:22
@serenibyss serenibyss added the status: high priority Issue or PR should be prioritized for reviews label Nov 23, 2021
@bruberu bruberu requested a review from Yefancy November 25, 2021 04:33
@Yefancy
Copy link
Member

Yefancy commented Nov 25, 2021

I don't know if I understand it right. when we create a mte in the world. we will create a PositionedSoundMTE, then put it into the SoundManager. Leave aside the efficiency of playing sound each tick. I didnt find any code to unload the PositionedSoundMTE. Is it always in the SoundManagereven if the machine is removed?

@Yefancy
Copy link
Member

Yefancy commented Nov 25, 2021

I may not have read the code carefully enough. If this is vanilla logic, it's too bad. I felt the need to implement our own sound system.

@Yefancy
Copy link
Member

Yefancy commented Nov 25, 2021

After careful reading I found that I was negligent, please ignore the comments above. I think it can be merged as soon as coremod issues be solved, and it worked fine when I tested it. really nice stuff

Copy link
Member

@Yefancy Yefancy left a comment

Choose a reason for hiding this comment

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

Because we add a few lines of classbytes, I don't know why other jump operators (e.g. goto) aren't broken, I remember should recalculating the position of the jump label. Probably my ASM learning is not in place, anyway its fine as long as it can run correctly. The final point is that there is no sound when using the wrench adjust the front face.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@bruberu bruberu merged commit e3e2960 into master Nov 26, 2021
@bruberu bruberu deleted the sounds branch November 26, 2021 17:57
@Yefancy Yefancy mentioned this pull request Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: high priority Issue or PR should be prioritized for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Suggestion] List of Needed Sounds [Suggestion] Machine sounds
5 participants