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

Debug Menu extensions: Display strings & interpreter stacks #3219

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

florianessl
Copy link
Member

Adds two new items to the F9 Debug Menu:

  • "Strings"
  • "Interpreter"

String display supports viewing multiline strings through a special new window "Window_StringView"
Two options can be toggled:

  • Automatic linebreaks
  • Command Evaluation ( default inserter with \v, \n, \t )

screenshot_160

Viewing the current interpreter stacks
(right side: [current command] / [commands_size])
screenshot_161

@Ghabry
Copy link
Member

Ghabry commented May 8, 2024

For the Linux build you must update the Makefile.am file similar to CMakeLists

@Ghabry
Copy link
Member

Ghabry commented May 8, 2024

Haven't read the entire code yet obviously. Just one thing I noticed:

You wrote your own wrap code. We already have a word wrapping function Game_Message::WordWrap. Maybe use this one if applicable?

@florianessl
Copy link
Member Author

Haven't read the entire code yet obviously. Just one thing I noticed:

You wrote your own wrap code. We already have a word wrapping function Game_Message::WordWrap. Maybe use this one if applicable?

I switched the code, so it uses this WordWrapping function now, instead of just cutting of at the exact position a text would overflow.

@florianessl
Copy link
Member Author

Rebased & refactored a bit. The ScopedVars branch depends on this, to make it possible to view & edit all frame-local variables for individual interpreter frames. I already moved some of the new code here (New choices window for debug scene) to make things easier to merge.

Considering the necessary changes for the debugging possibilities of the ScopedVars branch, this extension should be feature-complete, so feel free to test this!

@Ghabry
Copy link
Member

Ghabry commented May 14, 2024

Cool. We do not have too many games that use String Vars online.

This test game here could be suitable: https://easyrpg.org/play/pr3219/?game=string-var&test-play

I think I found a bug: In this game when I go to Interpreter -> Main no cursor appears.

However when I try e.g. uid https://easyrpg.org/play/pr3219/?game=uid&test-play it works. So I guess only having one interpreter bugs the Ui here?

… index. just always set it to 0 on PushUiInterpreterView; no need to remember the index here.
@Ghabry
Copy link
Member

Ghabry commented May 15, 2024

The issue is still not completely fixed: When you go in the string-var game to Strings -> Main the cursor is broken the first time. After Cancel -> Decision again it works 🤷

Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

My bar for Debug Scene changes is quite low as it cannot break code and is - well - for debugging. Imo that code can be merged as is after the two issues are discussed/resolved.

} else if (com.parameters[0] == 3 && Player::IsPatchManiac()) {
evt_id = Main_Data::game_variables->Get(com.parameters[1]);
} else if (com.parameters[0] == 4 && Player::IsPatchManiac()) {
evt_id = Main_Data::game_variables->GetIndirect(com.parameters[1]);
Copy link
Member

Choose a reason for hiding this comment

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

Are you attempting to figure out the event ID here?

Actually Maniac Patch added some debug fields to the ExexFrame for this purpose: maniac_event_info, maniac_event_id, maniac_event_page_id. We do not have this implemented and you do not have to implement everything but maniac_event_id could be useful to track the ID across maps because its not reset to 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's the idea..., I've noticed that there are several unimplemented fields, but skipped over them because I don't have any implementation details and didn't want to invest the time right now to figure this out. 😬
So this is far from perfect right now.
But yeah, I should probably change the tracked id field there. Completely forgot that the vanilla event_id gets reset.
I'm on holiday right now and won't have much time to look over this. If you want to change something, feel free to do so.

Copy link
Member

Choose a reason for hiding this comment

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

No Problem. Is nothing urgent Is enough when you annotate it with a fixme for now.

… map change) & "maniac_event_page_id" + extended interpreter debug view
- Base frame for CommonEvent interpreters was always shown with Id '0'
- Encountering a "CallEvent" command while traversing up the stack, always set the flag "is_calling_ev_ce", even if it was a map event
@florianessl
Copy link
Member Author

I added some "FIXME" comments for stuff that might need some minor rework, fixed some minor display issues & added support for the maniac fields "maniac_event_id" & "maniac_event_page_id".
I couldn't reproduce any more cursor issues with the strings example game though

Here's some example of a parallel process map event calling a CE, calling another map event, calling another CE...
screenshot_165
(Had to highlight this one, because this is some really unsafe stuff were it might be useful for the engine to warn the developer if such bounds are crossed -> on my list...)

src/window_interpreter.cpp Outdated Show resolved Hide resolved
Copy link
Member

@Ghabry Ghabry left a comment

Choose a reason for hiding this comment

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

Besides this one nit: LGTM 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants