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
Feature: "Remove all industries" button in scenario editor #8550
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea. I just quickly checked the code. And made some suggestions.
5e45870
to
770351b
Compare
Yeah it may be a good opportunity to remove the |
I agree, but I properly wouldn't put them on top of each other. Maybe next to each other is better suited? The word "industries" is a bit redundant here even, so, so "Build many random" and "Remove all" might be sufficient. Dunno, just thinking out loud :D |
Both changes would be very, very useful. They are sorely lacking.
It doesn't really matter much, but I think the buttons next to each other would look a bit worse due to the asymmetry of the buttons below. Also, full sentences, due to the importance of functions and their effects, seem justified here. Importantly, what is sorely lacking, and I would very much like to ask, is a confirmation request for both multi-enterprise construction and removal. There is nothing worse than pressing "many random" or "remove all" by accident after a long time creating a scenario ... It will be all the more important that after changing the "Remove all" button will be adjacent to the "Build". |
770351b
to
1e3b92f
Compare
Couldn't agree more. I pushed some additional changes. There's now a Create Random Industries button, I felt that caption was appropriate than "many random industries". Both the Remove and Create buttons ask for user confirmation. I kept the stacked buttons since I also think it will look strange if the bottom row contains a resize button. |
Looks very good. Removing the "hacky" Create Random Industries "industry" option is always good I do think the buttons would look better side-by-side though, rather than one above another |
How about with create/remove buttons at the top of the window? |
Honestly, while checking the preview (click the deployment button :D), I think on top of each other like you had it originally is totally fine (I know, I said otherwise earlier .. I changed my mind :P). Your fonts etc are making it look worse for me than it really is: This looks perfectly fine to me :) (well, maybe all buttons should be on top of each other, but meh, don't think that is for this PR to fix :D) |
It's basically a toss-up. Let's go with "top of the window, side by side" |
1e3b92f
to
c5063fe
Compare
This is how the window now looks on a clean configuration (so no custom fonts, no font/UI zoom) @LordAro : sure this is what you want? (honest question, but otherwise, lets merge this!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some coding-style nitpicks :)
c5063fe
to
073e894
Compare
@TrueBrain nitpicking addressed ;)
I agree that it might be better to put the buttons above each other. It already looks a little funky in English tbh, and I expect it to look worse in some "lengthier" languages. The main reason why I put the buttons above the industry list was to clearly show that they are independent of that list. Putting them below the list could create the impression that the industry selected is somehow going to affect what the buttons do, which is not the case. |
Did you forget to push them? :D Putting the buttons as pointed out in the last image makes absolutely no sense to me: And that settles it: at the top of the window, and on top of each other (not next to each other). It was a toss-up, but latest preview shows one clearly is sub-optimal :D Tnx for sticking with us @Kuhnovic , and sorry we are all nitpicking like this :D I really appreciate your effort here! |
073e894
to
ee45878
Compare
Forgot to stage my changes. Still getting the hang of git, I'm a mercurial guy ;) I can tolerate the nitpicking, better to think things through instead of going for a "fix it in the mix" later on :) I'll swap the orientation of the buttons now and commit that as well. And I'll try not to forget staging my changes this time ;) |
if you find yourself forgetting to stage all the time, you might use "git commit -a" instead, which automatically stages all changes. but there's very religious debates around that. |
ee45878
to
0b9379f
Compare
That sounds a little scary to me, but thanks for the tip! Everything is pushed now :) |
0b9379f
to
5ee17f3
Compare
Motivation / Problem
Many scenarios found in on the online content server come with industries. This is unfortunate when you want to play that particular map with for instance the FIRS industry set, or if you want to start with a clean slate for a different reason. The current solution is to go over the entire map and remove each industry, which is tedious.
Description
This feature is only available in the scenario editor. It removes all industries. It also turns removes residual farmland and replaces it with grass.
Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.