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

Remove librecomp usage from ultramodern #23

Merged
merged 13 commits into from
May 31, 2024

Conversation

AngheloAlf
Copy link
Collaborator

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 to ultramodern::events::callbacks_t::vi_callback
  • recomp::update_supported_options() changed to ultramodern::events::callbacks_t::gfx_init_callback

@AngheloAlf AngheloAlf linked an issue May 26, 2024 that may be closed by this pull request

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?
Copy link
Member

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.

ultramodern/src/events.cpp Show resolved Hide resolved
create_gfx_t* create_gfx;
create_window_t* create_window;
update_gfx_t* update_gfx;

Copy link
Member

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;
Copy link
Member

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.

@@ -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
Copy link
Member

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?
Copy link
Member

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".

#include "rsp.h"
#include "ultramodern/rsp.hpp"

static ultramodern::events::callbacks_t threads_callbacks{};
Copy link
Member

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"?

Copy link
Collaborator Author

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);
Copy link
Member

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.

@AngheloAlf AngheloAlf force-pushed the remove_librecomp_from_ultramodern branch from cb2f189 to d57405e Compare May 30, 2024 12:44
*
* 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);
Copy link
Member

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

@AngheloAlf AngheloAlf merged commit e3e7024 into main May 31, 2024
6 checks passed
@AngheloAlf AngheloAlf deleted the remove_librecomp_from_ultramodern branch May 31, 2024 02:56
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.

Remove ultramodern's dependency on librecomp
2 participants