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

Custom message override PoC #669

Closed

Conversation

garrettjoecox
Copy link
Contributor

@garrettjoecox garrettjoecox commented Jun 5, 2024

It's not perfect, but it works.

This isn't meant to be an end all solution, but aims to be short - medium term solution for custom messages. The idea is that you register your custom message in ShipMessages.h using the same format messages are extracted to in https://github.com/zeldaret/mm (after you have built the project look for message_data.h)

One thing that is absent, and still needs to be figured out is a good framework for replacing message contents, there is obviously built in control codes for this, but we'll want both more flexibility and more user friendliness than just using control codes. (eg templating like {{}})

We also need to bring in all of the message related changes from upstream so we can get all of the macros required to craft a message

Screenshot 2024-06-05 at 8 59 53 PM

Build Artifacts

@garrettjoecox garrettjoecox force-pushed the custom-message-poc branch 4 times, most recently from a52f5f8 to c2669eb Compare June 6, 2024 02:04
@garrettjoecox
Copy link
Contributor Author

It works 😄

Screenshot 2024-06-05 at 8 58 18 PM

@Marcelo20XX
Copy link

There is a program I use and its template is nice enough to work with, maybe you can take a look at it, it's name is Zelda64 Text Editor

2024-06-06.04-25-43.mp4

@Dumbledork01
Copy link

@garrettjoecox any updates on this? No rush if not. In the meantime, any recommendations on moving forward with the time selector enhancement?

@garrettjoecox
Copy link
Contributor Author

@garrettjoecox any updates on this? No rush if not. In the meantime, any recommendations on moving forward with the time selector enhancement?

Sorry, I’ve been out of pocket these last few weeks. I meant to remove the do not merge label after I fixed the remaining issue.

It’s still kind of a short/medium term PoC but I’d say it’s ready for people to use and try out. My intention was for you to use it and see if it allows you to do what you need, as a proving exercise. (Sorry I didn’t really convey this 😅)

@Dumbledork01
Copy link

Dumbledork01 commented Jun 26, 2024

Sorry, I’ve been out of pocket these last few weeks. I meant to remove the do not merge label after I fixed the remaining issue.

It’s still kind of a short/medium term PoC but I’d say it’s ready for people to use and try out. My intention was for you to use it and see if it allows you to do what you need, as a proving exercise. (Sorry I didn’t really convey this 😅)

@garrettjoecox No problem! I'll go ahead and use it this weekend and see how it works! Thanks for the update 👍🏿

@garrettjoecox
Copy link
Contributor Author

Any thoughts on this approach for medium term development (rando, anchor, etc)?

@inspectredc @Archez @Malkierian

@Archez
Copy link
Contributor

Archez commented Jul 22, 2024

I've been beginning to look through PRs starting from the back, but I can try to prioritize this one this evening.

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Ok, for a mid term solution, I think it works fine. There are some logistics that we can build on top of like you mentioned in the description.

In addition to custom control codes (these could even be custom per "mod" type), I imagine we will want a hook in one of the Message_Update methods for handling complex messages (3ds song of double time comes to mind).

Leaving an approval as is (assuming we wont be leaving the example text registered by time of merge).

mm/2s2h/CustomMessage/CustomMessage.cpp Outdated Show resolved Hide resolved
mm/2s2h/CustomMessage/CustomMessage.cpp Show resolved Hide resolved
mm/2s2h/Enhancements/Cheats/Cheats.h Outdated Show resolved Hide resolved
@garrettjoecox garrettjoecox force-pushed the custom-message-poc branch 3 times, most recently from 4b064c9 to d9922f8 Compare August 17, 2024 14:17
@garrettjoecox
Copy link
Contributor Author

@Archez made some small adjustments and added an example of text replacement.

GameInteractor_ExecuteOnHandleCustomMessage(activeMessageModId, activeMessageTextId, &msg);

// Copy modified message to the buffer
memcpy(buff + 11, msg.c_str(), msg.length());
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should have a check somewhere here to ensure that the resulting string after modification is within the buffer limit < (1280 - 11) so that the memcpy doesn't overflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@Archez Archez Aug 18, 2024

Choose a reason for hiding this comment

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

Yeah that works.

Although I'd prefer 1280 - 11 and 1280 - 11 - 1 respectively (or really defines for the max buffer and msg header size), but that's non-blocking for this PR (I'm sure we will be in this file a lot in the coming times)

Comment on lines +11 to +12
// Not really sure what the best ID is for this, but it needs to be between 0-255
// because it's used as a u8 somewhere in the chain
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiousity do u know where this is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One spot is the textId field in the GetItemEntry struct, might be other places

@garrettjoecox
Copy link
Contributor Author

Simplified and consolidated this work into #757 , may re-open separately later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants