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

Fix mesecons displayed as unknown(??) mod in the profiler #679

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

zmv7
Copy link
Contributor

@zmv7 zmv7 commented Aug 13, 2024

Currently there is the replacement of existing globalstep function, but it doesn't consider callback origins, therefore, the engine doesn't know what mod is the owner of the new function. The result - there is unknown entry in the profiler output, displayed as ?? modname with * instrumentation.

This patch fixes it by setting callback origin of the replaced globalstep function

@zmv7 zmv7 requested review from sfan5 and SmallJoker August 14, 2024 14:42
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Yes. LGTM.
If you wanted to optimize it even further, minetest.after might be an option to delay the globalstep registration. But then again, you'd also have to assign the function origin once.

@sfan5 sfan5 merged commit 368b294 into minetest-mods:master Aug 16, 2024
2 checks passed
@zmv7
Copy link
Contributor Author

zmv7 commented Aug 16, 2024

Yes. LGTM. If you wanted to optimize it even further, minetest.after might be an option to delay the globalstep registration. But then again, you'd also have to assign the function origin once.

Callbacks registered using minetest.after also displayed as ?? unfortunately

@zmv7 zmv7 deleted the patch-1 branch August 16, 2024 12:54
@SmallJoker
Copy link
Member

@zmv7 Hence the 2nd sentence, by which I was referring to minetest.callback_origins.

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