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

Introduce the RmlUI debugger #1845

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

necessarily-equal
Copy link
Contributor

@necessarily-equal necessarily-equal commented Jun 12, 2022

It has a companion PR on unvanquished_src.dpkdir to add keybindings for it.

Copy link
Contributor

@slipher slipher left a comment

Choose a reason for hiding this comment

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

Cool. It doesn't seem to work outside the main menu though. The buttons don't do anything (even if I open a menu so that I can click things).

@@ -432,6 +432,7 @@ static void CG_BeaconMenu_f()
Rocket_DocumentAction( rocketInfo.menu[ ROCKETMENU_BEACONS ].id, "show" );
}

void CG_RmlUIDebuggerToogle();
Copy link
Contributor

Choose a reason for hiding this comment

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

Toggle not Toogle

@@ -1184,6 +1185,8 @@ void CG_Init( int serverMessageNum, int clientNum, const glconfig_t& gl, const G
new(&cg) cg_t{};
for ( centity_t& ent : cg_entities) { ent = {}; }

CG_RmlUIDebuggerDisable(); // CHECKME
Copy link
Contributor

Choose a reason for hiding this comment

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

CG_Init is only called after a VM is freshly loaded so this is not needed here. Maybe you meant to put it in a shutdown function?

Copy link
Member

Choose a reason for hiding this comment

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

Further, I think rmlui will clean up the debugger by itself in the shutdown function. Thus, I think we can probably remove the function all together.

{
if (!debugger_initialised)
{
Rml::Debugger::Initialise(*cgs.mapname // CHECKME
Copy link
Member

Choose a reason for hiding this comment

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

You probably want an argument for hud/menu or at least allow for an override because people may want to debug ingame menus and such when a map is loaded. Furthermore, iirc, we never inject mouse/input events into the HUD context, so you can't actually use the debugger in that context (I think...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, wait, are the in-game menu part of the menu or HUD context?

Copy link
Member

Choose a reason for hiding this comment

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

Anything clickable is always the menuContext (ie, if you're in game and press ESC, you are in the menuContext). The hudContext is exclusively the the HUD (ie, things you can't click on because your mouse/keyboard are captured by the game. The only thing you you might actually want to click on is the tab scoreboard, which is part of the hud context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it makes things more complicated for the keybindings in particular. I'm not sure what would be the correct, non-surprising behavior. Use two keys? Press the same key several times (none->menu->hud->none)?

Copy link
Member

Choose a reason for hiding this comment

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

In games like CSGO, which have a similar scoreboard, if you open a clickable HUD element, you right click to bring up a mouse.

Clickable HUD elements might be interesting if we ever want to be cool like Doom3 and have clickable menus in the map.

{
if (!debugger_initialised)
{
Rml::Debugger::Initialise(*cgs.mapname // CHECKME
Copy link
Member

Choose a reason for hiding this comment

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

Also, once you pick a context, are you locked into that context forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the best I can do with current RmlUI, yes. More recent versions have introduced Rml::Debugger::Shutdown(); which would allow switching

Copy link
Member

Choose a reason for hiding this comment

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

Rml::Debugger::SetContext still exists and probably works.

Copy link
Member

Choose a reason for hiding this comment

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

Looking deeper, it seems we always want to initialize the debugger into the menuContext and then allow switching between the two via debuggerToggle menu or debuggerToggle hud. Still unsure if debuggerToggle will work with the hudContext though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it didn't work with the HUD, which is why I didn't notice that there was two contexts (EDIT: two contexts active at the same time)

Copy link
Member

Choose a reason for hiding this comment

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

For now, let's just enable the debugger for the menuContext and leave the hudContext as a TODO. This should make the code easier for now.

@necessarily-equal necessarily-equal marked this pull request as draft June 27, 2022 08:35
Copy link
Member

@DolceTriade DolceTriade left a comment

Choose a reason for hiding this comment

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

This is a good start. Let's finish this up and merge it? The debugger is actually quite a cool tool for debugging the menus.

if (debugger_initialised)
{
Rml::Debugger::SetVisible(false);
//Rml::Debugger::Shutdown(); // TODO(recent rmlui)
Copy link
Member

Choose a reason for hiding this comment

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

It's probably not terrible to leave the debugger loaded. After all, you only load it when you are debugging something and you'll probably want to debug something else.

{
if (!debugger_initialised)
{
Rml::Debugger::Initialise(*cgs.mapname // CHECKME
Copy link
Member

Choose a reason for hiding this comment

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

For now, let's just enable the debugger for the menuContext and leave the hudContext as a TODO. This should make the code easier for now.

@@ -1184,6 +1185,8 @@ void CG_Init( int serverMessageNum, int clientNum, const glconfig_t& gl, const G
new(&cg) cg_t{};
for ( centity_t& ent : cg_entities) { ent = {}; }

CG_RmlUIDebuggerDisable(); // CHECKME
Copy link
Member

Choose a reason for hiding this comment

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

Further, I think rmlui will clean up the debugger by itself in the shutdown function. Thus, I think we can probably remove the function all together.

@illwieckz illwieckz changed the base branch from 0.53.0/sync to master August 1, 2022 16:18
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.

None yet

3 participants