#7930 part2 add open folder button #7971

Osmodium · 2018-09-04T09:31:53Z

This is "part 2" of #7960 PR.

Added a button to the toolbar for easy access to the custom user content folder.

Currently only win32.

Osmodium · 2018-09-04T10:06:07Z

Did a "rebase" :)

Gymnasiast · 2018-09-04T10:56:06Z

Ok, macOS and Linux support will need to be added before this get merged. Other people can help with this.

In the case of Linux/BSD, I'd recommend using xdg-open to open the folder - it ensures the correct file manager is used.

Osmodium · 2018-09-04T10:59:54Z

I will have a look at macOS, but I do need help with the Linux part.

data/language/en-GB.txt

Gymnasiast · 2018-09-04T13:02:37Z

src/openrct2-ui/windows/TitleMenu.cpp

@@ -204,7 +204,7 @@ static void window_title_menu_dropdown(rct_window* w, rct_widgetindex widgetInde
                Editor::LoadTrackManager();
                break;
            case 4:
-                context_open_custom_user_content_folder();
+                context_open_custom_content_folder();


Hm, I see you followed a remark that I posted, but I removed that within a minute, since I wasn't sure. Sorry about that.

I'll look into it myself, since I'll have to add Linux support anyway.

Oh, I got an email about it, so I just thought I was blind haha.

Can just revert the commit if necessary :)

Nah, I'll do that. It's not worth refiring the CI just to remove the commit - I'll do it when I do the definitive rename and add Linux support.

Osmodium · 2018-09-04T13:04:15Z

To clarify why the mehod called is context_open_custom_content_folder and is part of context.h is due to the fact that TitleMenu.cpp doesn't include platform.h (where this method would belong in my opinion). So instead of including platform.h (and everything in it), I went ahead and added it to the context instead.
Also I added the method platform_open_folder to platform.h so there is a general method for this at a later point, though I can remove that, since it isn't used at the moment.

Gymnasiast · 2018-09-04T13:42:57Z

To clarify why the mehod called is context_open_custom_content_folder and is part of context.h is due to the fact that TitleMenu.cpp doesn't include platform.h (where this method would belong in my opinion). So instead of including platform.h (and everything in it), I went ahead and added it to the context instead.
Also I added the method platform_open_folder to platform.h so there is a general method for this at a later point, though I can remove that, since it isn't used at the moment.

@IntelOrca Could you say if this is the correct approach? I have a few doubts but I'm not sure enough to say so.

IntelOrca · 2018-09-04T13:55:15Z

You don't need a wrapper in Context, just call the UiContext method directly.
There is no further need to write context_ like methods as whole code base is C++.
platform_open_folder is also unnecessary.
Use ShellExecuteW to support unicode paths.

Osmodium · 2018-09-04T14:22:06Z

@IntelOrca I'm not sure I understand the first point.
It seems like I can't get the UiContext directly from TitleMenu.cpp which is why I went through the context.cpp wrapper. Should I just include it then, or is there something I'm missing?

IntelOrca · 2018-09-04T15:05:27Z

@Osmodium Just GetContext()->GetUiContext() should work. Make sure you have Context.h and UiContext.h included and that you are using namespace OpenRCT2.

Osmodium · 2018-09-04T15:34:05Z

@IntelOrca In order to make it work, I had to include both openrct2/ui/UiContext.h
openrct2/PlatformEnvironment.h and the implementation looks to me like something that should be somewhere else. But I guess if this is how you want it :)

EDIT: Though if I do this: OpenRCT2::GetContext()->OpenCustomUserContentFolder(); and keep the method in the context (where we already are including UIContext.h and PlatformEnvironment.h), we don't need to include anything in TitleMenu.cpp, which might be better?

IntelOrca · 2018-09-04T16:03:45Z

@Osmodium Yea, your screenshot is how I would have done it.

Osmodium · 2018-09-04T16:11:47Z

Okay, so it doesn't matter that we include 2 extra header files in the TitleMenu.cpp "just" to avoid having a method in Context.cpp that already has them included? Wouldn't this produce some sort of overhead, or am I overthinking it? :)

codecov-io · 2018-09-04T16:15:22Z

Codecov Report

Merging #7971 into develop will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff             @@
##           develop    #7971      +/-   ##
===========================================
- Coverage    57.31%    57.3%   -0.01%     
===========================================
  Files          572      572              
  Lines       239889   239899      +10     
===========================================
  Hits        137481   137481              
- Misses      102408   102418      +10

Impacted Files	Coverage Δ
src/openrct2/ui/UiContext.h	`50% <ø> (ø)`	⬆️
src/openrct2/Context.h	`100% <ø> (ø)`	⬆️
src/openrct2-ui/UiContext.h	`0% <ø> (ø)`	⬆️
src/openrct2-ui/UiContext.cpp	`0% <0%> (ø)`	⬆️
src/openrct2/Context.cpp	`20.81% <0%> (-0.08%)`	⬇️
src/openrct2-ui/UiContext.Linux.cpp	`0% <0%> (ø)`	⬆️
src/openrct2-ui/windows/TitleMenu.cpp	`0% <0%> (ø)`	⬆️
src/openrct2/ui/DummyUiContext.cpp	`17.64% <0%> (-0.27%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1fd033...cbda67f. Read the comment docs.

Gymnasiast · 2018-09-04T20:28:36Z

I have added Linux support. I also rebased your branch. A thing to note: never merge a branch into yours when doing a PR, always rebase.

Osmodium · 2018-09-04T21:05:50Z

@Gymnasiast Great and thanks! I haven't done so much rebasing, but I will do this in the future.

Gymnasiast

This is good to merge, as far as I'm concerned.

AaronVanGeffen · 2018-09-04T22:19:15Z

src/openrct2-ui/UiContext.Linux.cpp

@@ -113,6 +113,12 @@ namespace OpenRCT2::Ui
            }
        }

+        void OpenFolder(const std::string& path) override
+        {
+            std::string cmd = String::Format("xdg-open \"%s\"", path.c_str());


The argument needs to be shell-escaped.

It's already between double quotes, which is how we handle most cases. (And Zenity/dialog doesn't feature escaping at all!)

But if you think this escaping is insufficient, could you give more directions on how to do it, or add it yourself?

IntelOrca · 2018-09-05T19:57:46Z

src/openrct2-ui/UiContext.Linux.cpp

+            *outputPtr = '"';
+
+            return std::string(output);
+        }


If path is longer than 256 then you are going to exceed the fixed output buffer. I suggest creating a std::string of capacity 256 and appending to that.

Since you're passing a std::string by reference anyway, there's no need to do iterate the buffer the C way. Instead, you can do something like:

size_t index = 0; while ((index = path.find('"', index) != std::string::npos) { path.replace(index, 1, "\\\""); index += 2; }

Osmodium · 2018-09-11T11:00:51Z

Is there still an issue with the Linux way of getting the correct escaped path, or is is just the changelog that needs to be changed?

Gymnasiast · 2018-09-11T11:04:44Z

The Linux bit still has to be changed, but I don't quite know how to follow the instructions that @IntelOrca and @AaronVanGeffen gave.

… TitleMenu.cpp

AaronVanGeffen · 2018-09-13T20:34:17Z

I've applied my suggestion to simplify the EscapePathForShell implementation. While testing, I noticed the following:

The toolbox menu is now partially off-screen. Note it was offset like this before:

I have therefore moved the buttons up a little to compensate:

Osmodium · 2018-09-14T07:46:59Z

Beautiful! I don't know how I did not notice that it was a bit off-screen, but thanks for the adjustment :)

janisozaur · 2018-09-19T04:50:02Z

This broke Android builds. Please fix it.

Gymnasiast · 2018-09-19T07:06:19Z

Android build barely work if they build. Also, I'm sure you would be best person to do that job - you know most about it (along with Marijn).

IntelOrca · 2018-09-19T07:16:46Z

It is probably just a case of creating a stub for OpenFolder in the Android UI context.

- Feature: [#4418] Allow steep slopes on the side-friction roller coaster. - Feature: [#7726] Add shortcut to advance one tick. - Feature: [#7956, #7964] Add sprite font glyphs for Hungarian and some Czech letters. - Feature: [#7971] Toolbox option to open custom content folder. - Feature: [#7980] Allow data path for RCT1 to be specified by a command line argument. - Feature: [#8073] Auto-upload minidumps to backtrace.io (optional, MSVC/Windows only) - Feature: [#8078] Add save_park command to in-game console. - Feature: [#8080] New console variable "current_rotation" to get or set view rotation. - Feature: [#8098] Glyph for Russian rouble sign. - Feature: [#8099] Add Powered Launch mode to Inverted RC (for RCT1 parity). - Feature: [#8190] Allow building footpaths on 'corner down' terrain. - Feature: [#8191] Allow building on-ride photos and water S-bends on the Water Coaster. - Feature: [#8259] Add say command to in-game console. - Feature: [#8374] Add replay system. - Feature: [#8377] Add option to adjust amount of autosaves to keep. - Feature: [#8458] Add sprite sorting benchmark. - Feature: [#8583] Add boosters to water coaster. - Feature: [#8648] Add optional chat button to top toolbar in multiplayer games. - Feature: [#8652] Add network window including a graph for data usage visualisation. - Feature: [#8670] Add ability to download missing objects when loading a park. - Change: [#7961] Add new object types: station, terrain surface, and terrain edge. - Change: [#8222] The climate setting has been moved from objective options to scenario options. - Change: [#8718] Allow TARMAC object to be removed when running the `remove_unused_objects` command. - Change: [#8718] No longer require the generic scenery groups and tarmac footpath to be checked when creating a scenario. - Change: [#8734] Disable kick button in multiplayer window when unable to use it. - Fix: [#3832] Changing the colour scheme of track pieces does not work in multiplayer. - Fix: [#4094] Coasters with long flat-to-steep pieces offer them in diagonal mode (original bug). - Fix: [#5684] Player list can desync between clients and server and can crash. - Fix: [#6191] OpenRCT2 fails to run when the path has an emoji in it. - Fix: [#7439] Placement messages have mixed strings - Fix: [#7473] Disabling sound effects also disables "Disable audio on focus loss". - Fix: [#7536] Android builds fail to start. - Fix: [#7689] Deleting 0-tile maze gives a MONEY32_UNDEFINED (negative) refund. - Fix: [#7828] Copied entrances and exits stay when demolishing ride. - Fix: [#7945] Client IP address is logged as `(null)` in server logs. - Fix: [#7952] Performance drop caused by code refactor. - Fix: [#7954] Key validation fails on Windows due to non-ASCII user / player name. - Fix: [#7975] Inspection flag not cleared for rides which are set to never be inspected (original bug). - Fix: [#7985] Giant Screenshot ignores 'Map rendering' settings. - Fix: [#7987] Broken track designs increase money by MONEY32_UNDEFINED. - Fix: [#7991] Scenery and footpaths on Construction Rights tiles can be deleted using Clear Scenery. - Fix: [#8034] Vanilla sprites are broken when making screenshots from command line. - Fix: [#8045] Crash when switching between languages. - Fix: [#8062] In multiplayer warnings for unstable cheats are shown when disabling them. - Fix: [#8090] Maze designs saved incorrectly. - Fix: [#8101] Title sequences window flashes after opening. - Fix: [#8120] Crash trying to place peep spawn outside of map. - Fix: [#8121] Crash Renaming park with server logging enabled. - Fix: [#8139] Buying land costs money when the game is in "no money" mode. - Fix: [#8141] Attempting to build entrance/exit on station 2 does not work. - Fix: [#8142] Reliability of mazes and crooked houses can go below 100%. - Fix: [#8187] Cannot set land ownership over ride entrances or exits in sandbox mode. - Fix: [#8200] Incorrect behaviour when removing entrances and exits that are on the same tile. - Fix: [#8204] Crash when tile element has no surface elements. - Fix: [#8264] Rides and scenery placeable outside of map with ZC and Sandbox mode enabled. - Fix: [#8335] Rides with arbitrary ride types can crash the game when they break down. - Fix: [#8358] Infinite loop when changing vehicle count on stopped ride. - Fix: [#8402] Crash closing a window in some cases. - Fix: [#8431] Crash when game action logging is enabled. - Fix: [#8433] Crash if master server response is not valid JSON. - Fix: [#8434] Crash if curl_easy_init fails. - Fix: [#8443] Crash when selecting the current vehicle for ride that has none available. - Fix: [#8456] Junior booster track piece doesn't connect properly. - Fix: [#8464] Crash on game shutdown. - Fix: [#8469] Crash modifying colour on hacked rides. - Fix: [#8508] Underground roto-drop is not going up. - Fix: [#8555] Multiplayer window text limits are not computed properly. - Fix: [#8572] Steel Twister track pieces ID 64 and 65 drawn incorrectly. - Fix: [#8585] Part of track missing on air powered vertical coaster. - Fix: [#8588] Guest list scrolling breaks above ~2000 guests. - Fix: [#8591] Game loop does not run at a consistent tick rate of 40 Hz. - Fix: [#8647] Marketing campaigns check for entry fees below £1 (original bug). - Fix: [#8653] Crash when peeps attempt to enter a ride with no vehicles. - Fix: [#8720] Desync due to boats colliding with ghost pieces. - Fix: [#8739] Savegame from original game crashes when cruising through map. - Fix: [#8742] Access violation in vehicle_update_sound_params. - Fix: [#8804] Raising water shows money effect at the bottom rather than new height. - Fix: [#8811] Some fields in the sv6 save file not being copied correctly. - Fix: [#8824] Invalid read in footpath_chain_ride_queue. - Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only). - Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab. - Improved: [#7930] Automatically create folders for custom content. - Improved: [#7980] Show the full path of the scenario in the scenario select window. - Improved: [#7993] Allow assigning a keyboard shortcut for opening the tile inspector. - Improved: [#8107] Support Discord release of RCT2. - Improved: [#8491] Highlight entrance and exit with different colours in track design previews. - Improved: Almost completely new Hungarian translation. - Removed: [#7929] Support for scenario text objects.

AaronVanGeffen added the pending rebase PR needs to be rebased. label Sep 4, 2018

Osmodium force-pushed the 7930-part2-Add-open-folder-button branch from 673d432 to fbdd786 Compare September 4, 2018 09:41

Gymnasiast reviewed Sep 4, 2018

View reviewed changes

data/language/en-GB.txt Outdated Show resolved Hide resolved

AaronVanGeffen removed the pending rebase PR needs to be rebased. label Sep 4, 2018

Gymnasiast reviewed Sep 4, 2018

View reviewed changes

Gymnasiast force-pushed the 7930-part2-Add-open-folder-button branch from 70e234e to 75894a4 Compare September 4, 2018 20:27

Gymnasiast force-pushed the 7930-part2-Add-open-folder-button branch from 75894a4 to 3b704f6 Compare September 4, 2018 20:44

Gymnasiast added the changelog This issue/PR deserves a changelog entry. label Sep 4, 2018

Gymnasiast added a commit to Osmodium/OpenRCT2 that referenced this pull request Sep 4, 2018

Add OpenRCT2#7971 to changelog [ci skip]

b2bc019

Gymnasiast removed the changelog This issue/PR deserves a changelog entry. label Sep 4, 2018

Gymnasiast approved these changes Sep 4, 2018

View reviewed changes

Gymnasiast requested a review from IntelOrca September 4, 2018 21:20

AaronVanGeffen reviewed Sep 4, 2018

View reviewed changes

IntelOrca approved these changes Sep 4, 2018

View reviewed changes

IntelOrca reviewed Sep 5, 2018

View reviewed changes

Osmodium and others added 8 commits September 13, 2018 22:18

Added 'open custom content folder' button to toolbar menu.

48d6e2f

Added MacOS OpenFolder support.

6e6753a

Removed c styled method.

97ebf50

Removed methods from Context, added headers and functionality only in…

b1ac769

… TitleMenu.cpp

Add Linux support

40cd64c

Rename STR_OPEN_CUSTOM_USER_FOLDER

dfdb3b4

Add OpenRCT2#7971 to changelog [ci skip]

3623e80

Add function to shell-escape path

e843483

AaronVanGeffen force-pushed the 7930-part2-Add-open-folder-button branch from f855a17 to 07b9104 Compare September 13, 2018 20:31

AaronVanGeffen force-pushed the 7930-part2-Add-open-folder-button branch 3 times, most recently from 40b525c to 9731aa3 Compare September 13, 2018 20:59

Gymnasiast approved these changes Sep 13, 2018

View reviewed changes

AaronVanGeffen added 2 commits September 13, 2018 23:20

Simplify EscapePathForShell by using std functions.

e1d6585

Move title menu buttons up to compensate for new item.

d6769fa

AaronVanGeffen force-pushed the 7930-part2-Add-open-folder-button branch from 9731aa3 to d6769fa Compare September 13, 2018 21:20

AaronVanGeffen merged commit a5ad9a3 into OpenRCT2:develop Sep 13, 2018

AaronVanGeffen mentioned this pull request Sep 13, 2018

String 6266 OpenRCT2/Localisation#1557

Closed

18 tasks

Osmodium deleted the 7930-part2-Add-open-folder-button branch September 14, 2018 10:55

janisozaur added a commit to janisozaur/OpenRCT2 that referenced this pull request Sep 20, 2018

Fix Android builds after OpenRCT2#7971

d1c871d

janisozaur added a commit that referenced this pull request Sep 20, 2018

Fix Android builds after #7971 (#8009)

fbb71c7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#7930 part2 add open folder button #7971

#7930 part2 add open folder button #7971

Osmodium commented Sep 4, 2018 •

edited

Loading

Osmodium commented Sep 4, 2018

Gymnasiast commented Sep 4, 2018

Osmodium commented Sep 4, 2018

Gymnasiast Sep 4, 2018

Osmodium Sep 4, 2018

Osmodium Sep 4, 2018

Gymnasiast Sep 4, 2018

Osmodium commented Sep 4, 2018 •

edited

Loading

Gymnasiast commented Sep 4, 2018

IntelOrca commented Sep 4, 2018 •

edited

Loading

Osmodium commented Sep 4, 2018 •

edited

Loading

IntelOrca commented Sep 4, 2018

Osmodium commented Sep 4, 2018 •

edited

Loading

IntelOrca commented Sep 4, 2018

Osmodium commented Sep 4, 2018

codecov-io commented Sep 4, 2018

Gymnasiast commented Sep 4, 2018

Osmodium commented Sep 4, 2018

Gymnasiast left a comment

AaronVanGeffen Sep 4, 2018

Gymnasiast Sep 5, 2018

IntelOrca Sep 5, 2018

AaronVanGeffen Sep 5, 2018 •

edited by Broxzier

Loading

Osmodium commented Sep 11, 2018

Gymnasiast commented Sep 11, 2018

AaronVanGeffen commented Sep 13, 2018 •

edited

Loading

Osmodium commented Sep 14, 2018

janisozaur commented Sep 19, 2018

Gymnasiast commented Sep 19, 2018

IntelOrca commented Sep 19, 2018

#7930 part2 add open folder button #7971

#7930 part2 add open folder button #7971

Conversation

Osmodium commented Sep 4, 2018 • edited Loading

Osmodium commented Sep 4, 2018

Gymnasiast commented Sep 4, 2018

Osmodium commented Sep 4, 2018

Gymnasiast Sep 4, 2018

Choose a reason for hiding this comment

Osmodium Sep 4, 2018

Choose a reason for hiding this comment

Osmodium Sep 4, 2018

Choose a reason for hiding this comment

Gymnasiast Sep 4, 2018

Choose a reason for hiding this comment

Osmodium commented Sep 4, 2018 • edited Loading

Gymnasiast commented Sep 4, 2018

IntelOrca commented Sep 4, 2018 • edited Loading

Osmodium commented Sep 4, 2018 • edited Loading

IntelOrca commented Sep 4, 2018

Osmodium commented Sep 4, 2018 • edited Loading

IntelOrca commented Sep 4, 2018

Osmodium commented Sep 4, 2018

codecov-io commented Sep 4, 2018

Codecov Report

Gymnasiast commented Sep 4, 2018

Osmodium commented Sep 4, 2018

Gymnasiast left a comment

Choose a reason for hiding this comment

AaronVanGeffen Sep 4, 2018

Choose a reason for hiding this comment

Gymnasiast Sep 5, 2018

Choose a reason for hiding this comment

IntelOrca Sep 5, 2018

Choose a reason for hiding this comment

AaronVanGeffen Sep 5, 2018 • edited by Broxzier Loading

Choose a reason for hiding this comment

Osmodium commented Sep 11, 2018

Gymnasiast commented Sep 11, 2018

AaronVanGeffen commented Sep 13, 2018 • edited Loading

Osmodium commented Sep 14, 2018

janisozaur commented Sep 19, 2018

Gymnasiast commented Sep 19, 2018

IntelOrca commented Sep 19, 2018

Osmodium commented Sep 4, 2018 •

edited

Loading

Osmodium commented Sep 4, 2018 •

edited

Loading

IntelOrca commented Sep 4, 2018 •

edited

Loading

Osmodium commented Sep 4, 2018 •

edited

Loading

Osmodium commented Sep 4, 2018 •

edited

Loading

AaronVanGeffen Sep 5, 2018 •

edited by Broxzier

Loading

AaronVanGeffen commented Sep 13, 2018 •

edited

Loading