-
-
Notifications
You must be signed in to change notification settings - Fork 957
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: Selective demolition tool. #7497
Conversation
6285a0c
to
50440f7
Compare
254980a
to
ae00181
Compare
How could be the rising button problem solved? Currently can not abort the demolition if a player clicks on the button again, only with the ESC key. |
929f359
to
78b09fe
Compare
I have reworked the GUI part: instead of a drop-down list, it is now using a small toolbar window. That way the above mentioned problem is solved. |
78b09fe
to
9fb7be5
Compare
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.
Code looks more or less fine, not sure about the concept though
EndContainer(), | ||
NWidget(NWID_HORIZONTAL), | ||
NWidget(WWT_IMGBTN, COLOUR_DARK_GREEN, WID_DM_ALL), SetMinimalSize(22, 22), | ||
SetFill(0, 1), SetDataTip(SPR_IMG_DYNAMITE, STR_DEMOLISH_EVERYTHING_TOOLTIP), |
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.
too much indentation here (though I'm not sure what the "expected" amount is, only that 4 tabs is too much)
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 am also not sure how much is expected, but I checked that file when I wrote it, and around line 370 there are the same amount of tabs where the widget attributes are in multiple lines, so I wanted to be consistent with them.
0d42e67
to
82c0007
Compare
82c0007
to
97801b1
Compare
Hello :) is there any more work that has to be done with this tool? |
97801b1
to
d7a3ef5
Compare
Hi @stormcone , First off, thank you for this PR. Second, sorry it took us this long to get back to you about what we think about this PR :) Basically, we are all a bit so-so about this functionality. Not because the concept is bad, or the code is bad, but because it only solves a tiny part of a bigger puzzle. Let me explain a bit: I can fully understand that the current "destroy everything" is not ideal. I often find too that I only want to destroy this or that .. mostly, I mostly do not want to destroy water. So I get where this is coming from. The implementation however, raises a lot of questions, and we expect our users will have the same. For example, why can I remove trees, but not all stations? Or all town roads? Or only rivers? Or everything except rivers? There are so many combinations and options possible with this, that it is unclear to us what the intentions behind only these 2 options are. For me personally, it wouldn't be enough .. and this is what I am afraid of: that after this PR, we will get a lot of small requests to add this or that to the destroy bar .. ruining the bar in no time :D So, we will need to look for another way to do this. As the concept is nice, in my opinion, just the way it is implemented will lead to a lot of questions and frustration with our users. How an alternative implementation should look? I really do not know .. if we look at other games, we see that they solve it in all kinds of different ways .. for example, Factorio uses blueprints for that, but we don't have those. So maybe someone can come up with an alternative way to supply this functionality, that does allow all the weird requests people will have "I only want to demolish X" :D I hope you understand; but I am going to close this PR. As it stands, this PR is just not the solution for us .. I do hope you can come up with an alternative approach, which does allow all this fine-grained control :) If you are interested, I suggest to open up a Discussion to talk over what others think or how we should approach is. Again, sorry it took us almost 2 years to give you this reply, and I seriously appreciate your work here. But sometimes we have to think about the greater picture here :) Tnx again! |
Idea from #7394.
Known bug(?):
The demolish button does not stay lowered because the drop-down list window raises back up when it closes. And I do not know how to keep it down. Maybe I need to get a paperweight. :)