Uh oh!
There was an error while loading. Please reload this page.
ModManagerGUI redo v2 - #268
Conversation
HeliumAnt
left a comment
There was a problem hiding this comment.
Amazing work! Fixes needed for linux/macos, also some questionable code style decisions.
| void ModManagerGUI::OpenModsFolder() { | ||
| static const char folder[] = ".\\Mods"; | ||
| #if defined(_WIN32) | ||
| ShellExecuteA(nullptr, "open", folder, nullptr, nullptr, 10); | ||
| // TODO: Confirm this works on Mac and Linux | ||
| #elif defined(__APPLE__) | ||
| system(("open \"" + folder + "\"").c_str()); | ||
| #elif defined(__linux__) | ||
| system(("xdg-open \"" + folder + "\" &").c_str()); | ||
| #endif |
There was a problem hiding this comment.
move to System, please. Also cstrings can't be concatenated like this, use std::string, std::filesytstem::path or std::format instead. Maybe use SDL_OpenURL for linux/macos (may only work with absolute paths)
There was a problem hiding this comment.
Thank you, will! Going to note that i won't be able to check this on linux and macos
| static auto toggleToggleEntryButton = [&](bool enable) { | ||
| m_ToggleListEntryButton->SetVisible(enable); | ||
| m_ToggleListEntryButton->SetEnabled(enable); | ||
| }; | ||
| static auto resetRightHandSide = [&]() { | ||
| UpdateModuleHeader(-1); | ||
| m_ModOrScriptDescriptionLabel->SetText(GetDisclaimerText()); | ||
| m_ToggleListEntryButton->SetVisible(false); | ||
| m_ToggleListEntryButton->SetEnabled(false); | ||
| toggleToggleEntryButton(false); | ||
| }; |
There was a problem hiding this comment.
just make these (private) member functions.
| } | ||
| // Unique_ptr so it gets GC'd | ||
| std::unique_ptr<BITMAP> newIconBm8Streched(create_bitmap_ex(8, maxSize, maxSize)); |
There was a problem hiding this comment.
use std::unqiue_ptr<BitmapDeleter, BITMAP> (BitmapDeleter is currently with FrameMan iirc) otherwise the bitmap isn't deallocated properly.
There was a problem hiding this comment.
This is new to me and im glad to learn this, i'll look into it and change this
| @@ -14,9 +15,15 @@ | |||
| #include "GUIButton.h" | |||
| #include "GUIListBox.h" | |||
There was a problem hiding this comment.
missing <algorithm> and <cctype> include
There was a problem hiding this comment.
Huh! Maybe that got left in another commit, but doubting. It ran for me, will look at it!
| std::string formattedStr = module->GetFriendlyName(); | ||
| if (formattedStr == "") { | ||
| formattedStr = g_PresetMan.GetDataModuleName(moduleIndex); | ||
| transform(formattedStr.begin(), formattedStr.end(), formattedStr.begin(), ::tolower); |
There was a problem hiding this comment.
explicit std::transform and std::tolower (below as well). pretty sure this shouldn't work without that actually.
| if (selectedModuleIconBm->h == 0 || selectedModuleIconBm->w == 0) { | ||
| return; | ||
| } | ||
| float aspect = float(selectedModuleIconBm->w) / float(selectedModuleIconBm->h); |
There was a problem hiding this comment.
bad practice, use static_cast please. this is just a horrendous way to write a c-style cast
There was a problem hiding this comment.
I just freestyle'd here to get it over with, i'll change 😁
ModManagerGUI.mp4
Building on previous changes that grouped scripts by collapsible module categories.
This PR also adds glyphs for digits, apostrophe, dash, period and underscore to the title screen font, here used for module titles.