-
Notifications
You must be signed in to change notification settings - Fork 7
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
Updated OnDrawingFinished hook to not be overwritten by other mods. #201
Conversation
source/CGameMenu.cpp
Outdated
@@ -24,6 +24,8 @@ namespace CLEO | |||
|
|||
void __fastcall OnDrawMenuBackground(void *texture, int dummy, RwRect2D *rect, RwRGBA *color) | |||
{ | |||
GetInstance().Start(true); // late initialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it called from here and not from OnScmInit1 for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be very late. If level 3 of logging is enabled there are already messages to display in main menu.
Future ambient scripts will also log stuff right after game starts.
CodeInjector.ReplaceFunction(OnFlushObrsPrintfs, address); | ||
break; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be a separate method, no need to increase complexity of an already long function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have single initialization function instead of two. After more thinking it should probably be made with variable int initializationStage
, and incremented. So next stage can not be performed until previous was executed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to over-engineer simple stuff. please create a new method called setupOnDrawingHook
and call it separately from the core init.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. There will be more things to initialize late. I had exact same need to start ModLoader system after all other asi plugins are loaded.
Hooking read string param function also can be considered to perform later.
Maybe that whole logic should be wrapped into SmartHook class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
3c68bce
to
2f4211a
Compare
Added late initialization phase for installing on drawing finished hook.
Fixes #169
Fixes missing prints with SkyGFX mnod installed too.