Ambient icon indicating copy to clipboard operation
Ambient copied to clipboard

Add "Dump UI" button to the debug menu

Open philpax opened this issue 2 years ago • 3 comments

When you're in an Ambient game with a client, there are three ECS worlds: the UI world, the client world, and the server world. The debugger offers buttons to dump the Client World and the Server World, but does not offer a button to dump the UI world. This is something we accidentally left out.

If you'd like to get your feet wet with our UI system and/or get an understanding of how the before it lands in the guest API, you can have a go at trying to add the button itself. The main thing you need to do is to clone this:

https://github.com/AmbientRun/Ambient/blob/41a6f86c3df12707a02a72b6f4c660fc63cfe6c0/crates/debugger/src/lib.rs#L56-L65

and dump the world passed in in the Button, and not in get_state (which gets the client world).

Things to watch out for:

  • The hotkeys. They're already wrong (Show entities and DCW use the same hotkeys) - feel free to go ahead and reassign them.
  • dump_world_hierarchy_to_tmp_file doesn't take a path. You should update this.
    • If you want to take this further: update all of the dump methods to use a unified code path for writing files, so that they all behave consistently w/r/t logging and output location. This may also be a good opportunity to add a button that dumps all of the state at the same time for easy debugging.

Feel free to ask any questions!

philpax avatar Mar 01 '23 18:03 philpax

I'll give it a go!

owenpalmer avatar Mar 02 '23 06:03 owenpalmer

@philpax Okay I got the new button dumping the UI world state.

  1. As for the custom file path on dump_world_hierarchy_to_tmp_file, should all world_dumps go to the tmp directory, but allow for subdirectories within it?
  2. You mentioned a unified code path for writing to files, is this referring to all dumps or just world dumps?
  3. For the hotkeys, I was thinking they could be from F1-F8, starting in order from "Show Entities" on the right to "Shader Debug" on the left. Thoughts?

owenpalmer avatar Mar 02 '23 08:03 owenpalmer

@philpax Okay I got the new button dumping the UI world state.

1. As for the custom file path on dump_world_hierarchy_to_tmp_file, should all world_dumps go to the tmp directory, but allow for subdirectories within it?

2. You mentioned a unified code path for writing to files, is this referring to all dumps or just world dumps?

3. For the hotkeys, I was thinking they could be from F1-F8, starting in order from "Show Entities" on the right to "Shader Debug" on the left. Thoughts?
  1. Either is fine, really - I'd just take a &str argument for the dump name, and then have it go to some directory. tmp/ is fine for now, but maybe we'll want to change it later? e.g. just have a write_debug_file(name: &str, contents: &[u8]) function
  2. Anything that can be produced by the debug buttons would be good to have in one place, so you always know where to go and what to look for.
  3. Seems reasonable to me! In future we might want to have submenus and to nest these, but that should be totally sufficient for now.

philpax avatar Mar 02 '23 10:03 philpax

Hi @philpax, looking into changing the hotkeys has led me down a little rabbit hole.

The hotkey to "Show Shadow Frustums" wasn't triggering the closure passed to the button, which eventually led me to this, which actually maps a single key to those functions. It doesn't call the on_invokes, which makes me think this code shouldn't exist? Was there another purpose for it? https://github.com/AmbientRun/Ambient/blob/f94fd3d7b00c89d591f73449c28a41b68a82c0e3/crates/app/src/lib.rs#L608-L613

There was a bug with the existing hotkey and hotkey_modifier methods for the buttons, which I'll push a fix for along with the new UI button. Had to do with the modifers never being populated.

Let me know if you need/want more details.

owenpalmer avatar Mar 06 '23 04:03 owenpalmer

Interesting, thanks! I'm not actually sure myself - that's a question for @FredrikNoren, who can hopefully shed some light on the intended behaviour.

philpax avatar Mar 06 '23 08:03 philpax

@owenpalmer So that code in the ExamplesSystem is just for all the native examples (like https://github.com/AmbientRun/Ambient/blob/main/crates/renderer/examples/instancing.rs). The hotkeys in the top bar should just be set using the hotkey method on Button.

That said, the UI system is in a bit of flux right now as we're trying to get it to run in guest code too, so there might be bugs.

FredrikNoren avatar Mar 06 '23 08:03 FredrikNoren

Okay that makes sense. What should I do with those additional hotkey mappings? Simply removing them gives warnings of not using world, dump_to_tmp_file(), and dump(). Both of those dump functions are defined in crates\app\src\renderers.rs. There seems to be a lot of different methods and functions that dump to the tmp dir. Philpax was talking about having a unified code path for that.

owenpalmer avatar Mar 06 '23 16:03 owenpalmer

Yeah, what I'd suggest is defining a function in ambient_std or something like ambient_std::write_dump(filename: &str, contents: &[u8]) (or &str if that works), and then changing all of the existing functions that write to tmp to use that instead. String implements Write, so you should be able to adapt the code that writes to files by writing to a string instead.

philpax avatar Mar 06 '23 16:03 philpax