Port Display List Viewer#1310
Conversation
Eblo
left a comment
There was a problem hiding this comment.
One thing that would be nice is the ability to limit the DL list to those active in the current scene. However, DLs remain persistent in memory once loaded, and I don't think there is currently tracking for what is in active use, so I would say that's out of scope for this PR.
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
| #include <string> | ||
| #include <cstdint> | ||
| #include <algorithm> | ||
| #include <libultraship/libultraship.h> |
There was a problem hiding this comment.
You probably don't need this entire import.
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
| PerformDisplayListSearch(); | ||
| } | ||
|
|
||
| searchDebounceFrames--; |
There was a problem hiding this comment.
What is a "frame" in this context? Is it based on refresh rate? For higher refresh rates, 30 frames may not be meaningfully long enough to debounce.
There was a problem hiding this comment.
This is a remnant from SoH. Gonna move to a time-based debounce instead. Similar to BenInputEditorWindow.
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
| if (ImGui::Selectable("gsDPSetGrayscaleColor")) { | ||
| *gfx = gsDPSetGrayscaleColor(0, 0, 0, 255); | ||
| } | ||
| ImGui::EndCombo(); |
There was a problem hiding this comment.
Could we throw in gsSPNoOp? I think effectively canceling out a command in place will be useful, going by previous experiences in patching graphical commands.
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
| ImGui::SameLine(); | ||
| UIWidgets::PushStyleCheckbox(THEME_COLOR); | ||
| if (ImGui::Checkbox(("state" + id).c_str(), state)) { | ||
| // |
There was a problem hiding this comment.
I'll remove it. You're guess is as good as mine.. this was directly copy and pasted from SoH lol
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
|
|
||
| void PerformDisplayListSearch() { | ||
| auto result = Ship::Context::GetInstance()->GetResourceManager()->GetArchiveManager()->ListFiles( | ||
| "*" + std::string(searchString) + "*DL*"); |
There was a problem hiding this comment.
This search is case sensitive. Is that intended?
There was a problem hiding this comment.
Another copy/paste from SoH. It looks like they may have intended it to not be. Glob_match appears to be case sensitive though, so what I'll do is convert everything to lowercase before comparing.
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
| UIWidgets::PushStyleCombobox(THEME_COLOR); | ||
| if (ImGui::BeginCombo(("CMD" + id).c_str(), cmdLabel.c_str())) { | ||
| if (ImGui::Selectable("gsDPSetPrimColor") && cmd != G_SETPRIMCOLOR) { | ||
| *gfx = gsDPSetPrimColor(0, 0, 0, 0, 0, 255); |
There was a problem hiding this comment.
If I'm following this right, you're actually irreversibly modifying the DL commands in place, meaning they will remain modified for the remainder of runtime. I think a better approach would be to call something like this, where the last argument is what you normally do to gfx now:
ResourceMgr_PatchGfxByName(activeDisplayList.c_str(), ("CMD" + id + "Debug").c_str(), id, gsDPSetPrimColor(0, 0, 0, 0, 0, 255));One benefit to doing this is that you could add a revert button that calls this and restores the default DL command:
ResourceMgr_UnpatchGfxByName("scenes/nonmq/Z2_00KEIKOKU/Z2_00KEIKOKU_room_00DL_00D490", ("CMD + id + "Debug").c_str());
mm/2s2h/DeveloperTools/DLViewer.cpp
Outdated
| std::string GetCommandMetadata(Gfx* gfx, int cmd, size_t currentIndex, size_t totalInstructions) { | ||
| std::string metadata; | ||
|
|
||
| if (cmd == G_SETTILE) { |
There was a problem hiding this comment.
This should probably be a switch block.
There was a problem hiding this comment.
Good call. I went ahead and refactored it. That change is now in.
As the title says. The tool supports viewing various graphics commands (textures, vertices, colors, etc.) and allows live editing of color-related commands.
Build Artifacts