-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove librecomp usage from ultramodern #23
Conversation
ultramodern/src/error_handling.cpp
Outdated
|
||
void ultramodern::error_handling::message_box(const char* msg) { | ||
// We print the message to stderr since the user may not have provided a message_box callback | ||
// TODO: is fprintf ok? or do we prefer using something more C++'ish? |
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.
fprintf is OK here. If anything in the long run this may add fmtlib as a dependency for type-safe printing, but this is plenty good enough for now.
create_gfx_t* create_gfx; | ||
create_window_t* create_window; | ||
update_gfx_t* update_gfx; | ||
|
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.
A destroy_ui callback isn't actually needed, it's handled by the RT64 render hook API.
RspUcodeFunc* ucode_func = rsp_callbacks.get_rsp_microcode(task); | ||
|
||
if (ucode_func == nullptr) { | ||
return false; |
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.
A stderr print would make sense here for easier debugging while developing.
librecomp/src/recomp.cpp
Outdated
@@ -499,6 +505,13 @@ void recomp::start(ultramodern::WindowHandle window_handle, const ultramodern::a | |||
gfx_callbacks.update_gfx(gfx_data); | |||
} | |||
} | |||
|
|||
// moved from ultarmodern/src/events.cpp/gfx_thread_func |
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 isn't needed, see my comment on the gfx_callbacks struct change.
@@ -132,6 +136,7 @@ struct audio_callbacks_t { | |||
set_frequency_t* set_frequency; | |||
}; | |||
|
|||
// TODO: This really isn't used by ultramodern. Should we move it to librecomp instead? |
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.
The plan is to move the osCont functions into ultramodern and then change the existing librecomp osCont functions into translation funcs, so this TODO can instead say something like "These functions are currently called by librecomp, but will get called by ultramodern in the future".
ultramodern/src/events.cpp
Outdated
#include "rsp.h" | ||
#include "ultramodern/rsp.hpp" | ||
|
||
static ultramodern::events::callbacks_t threads_callbacks{}; |
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.
Any reason this is called "threads_callbacks" instead of "events_callbacks"?
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.
Because I originally called that namespace threads
and renamed it to events
, but forgot to rename the variable 😅
void set_callbacks(const callbacks_t& callbacks); | ||
|
||
void init(); | ||
bool run_microcode(RDRAM_ARG const OSTask* task); |
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.
I think this would be better named "run_task", since the underlying implementation may not be a recompiled microcode for some projects.
…e rsp_callbacks_t
cb2f189
to
d57405e
Compare
* | ||
* It must be called only once and it must be called before `ultramodern::preinit`. | ||
*/ | ||
void set_callbacks(const rsp::callbacks_t& rsp_callbacks, const audio_callbacks_t& audio_callbacks, const input_callbacks_t& input_callbacks, const gfx_callbacks_t& gfx_callbacks, const events::callbacks_t& thread_callbacks, const error_handling::callbacks_t& error_handling_callbacks); |
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.
Missed a "thread_callbacks" -> "events_callbacks" rename here
I accomplished this by adding many callbacks.
The most notable case being the RSP one, where ultramodern requires a callbacks to actually execute the RSP microcode. librecomp provides this callback, but it also requires another callback (provided by the game itself) to get the microcode function to execute based on the given task.
Other notable changes are:
recomp::update_rumble()
changed toultramodern::events::callbacks_t::vi_callback
recomp::update_supported_options()
changed toultramodern::events::callbacks_t::gfx_init_callback