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

Entity vtable fixes and new stuff #397

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

Mr-Auto
Copy link
Contributor

@Mr-Auto Mr-Auto commented Sep 11, 2024

No description provided.

…e, add missing parameters, make the vtable hooks not require giving the full signature, instead you can point to the function pointer. Fix auto doc (again)
Comment on lines +263 to +265
// there is actually base class that consists of up to the left/right channel, used for the sfx
// then normal sounds that you can find in entities, screens itp. have up to the paddings
// finally music in GameManages has the extra bool in BackgroundSound
Copy link
Contributor Author

@Mr-Auto Mr-Auto Sep 11, 2024

Choose a reason for hiding this comment

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

Should i split this? (if yes, would love some name suggestions)

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it would be nice if it means more of those sound types exposed. But I don't really know about name, since now I guess we only have to use a name without altering the SoundMeta one...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can edit the name, as long as there is no new constructor exposed to lua, no one should really care about the metatable name

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I can't come up with any name ideas, since I'm not completely sure what's the difference between those clases 🐈.
Is the current SoundMeta for loop-able long sounds? Having a bool for play_ending_secuence makes it sound like it's repeatable and can be ended by game logic, and also the start_over

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you could leave it for another PR if you wanted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The start_over is more of a name i give it to make sense in the API rather then what actual functionality it holds.
The class name doesn't need to reflect much. In the source code it's probably just base class like "Sound".

That base class is used for stuff like: play and forget. You can just play sfx and not worry about it as the internals of it will deal with cleaning after it finishes etc.
Then SoundMeda is for sounds you want you either reuse or need to maintain (like keep updating the position etc.), it also used when you need to know if previous sound is still playing. Pretty sure you need to call the destroy function on it yourself

At the end is BackgroundSound which is probably only implemented for the main "music" in game_manager

There might be more that we don't know of, or that don't differ by the class members (only virtual overrides)

src/game_api/vtable_hook.hpp Outdated Show resolved Hide resolved
src/game_api/entity.cpp Outdated Show resolved Hide resolved
… and vice versa. Some comments fix, clean up Entity file a little,expose some more stuff
…t them etc. some comments and name changes, filter out bad pointers in vtable_sizes generated by info_dump
@Mr-Auto Mr-Auto marked this pull request as ready for review November 16, 2024 12:38
Copy link
Contributor

@estebanfer estebanfer left a comment

Choose a reason for hiding this comment

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

Most of it is great! Just some small things to fix

src/game_api/entities_items.hpp Outdated Show resolved Hide resolved
src/game_api/entities_monsters.hpp Outdated Show resolved Hide resolved
src/game_api/movable.hpp Show resolved Hide resolved
src/game_api/entities_mounts.hpp Outdated Show resolved Hide resolved
src/game_api/script/usertypes/vtables_lua.cpp Outdated Show resolved Hide resolved
src/game_api/script/usertypes/vtables_lua.cpp Show resolved Hide resolved
…ble function that have different name for hook vs function itself, some comments here and there
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.

3 participants