Make Terrain Brush "full tile" painting easier to find

[bjorn]

The Terrain Brush can fill full tiles by holding Control. It is mentioned in the manual, but it would be nice if this feature was easier to discover. Also some people may want this mode to be the default, so that they don't have to hold Control all the time.

I'd suggest we add a toggle button, or two buttons to switch between, to the toolbar for the Terrain Brush. It should allow changing the current mode persistently, and Control can remain for temporarily switching to the other mode.

[eishiya]

If a toggle button or mode selector is added, I think the old full-tile behaviour should be the default, for consistency with older versions of Tiled. The toggle would make it easy to get the corner/edge-modifying behaviour for those that need it, but full-tile as the default would mean people coming from old versions of Tiled or following old tutorials wouldn't be confused.

Edit: It occurs to me now that the old behaviour only makes sense as a default for non-Edge terrains. So, the current default of affecting the current edge/corner is fine.

[dnc77]

Hi there,

I decided to look at this project and consider this as a first fix. After some reviews on my end, I created a drop down and an option to enable/disable full tile mode. It definitely needs to be reviewed as I am still not 100% familiar with the project but it will look like the attached. Would you be happy to review this? Is the process right; to fork the repo, put in the change and commit it in the fork if you are happy to review this or is there another process please?

Cheers

Duncan

[eishiya]

This should not be a dropdown, but a tool option, similar to the random/terrain options on the Bucket Fill tool. There was also an icon for the toolbar already created on Discord, @bjorn presumably has it.

Edit: The work's already been done in the linked PR though, all it's missing is that icon I think. There's probably no need for a second fork for the same feature, unless you found a better way to do it than the linked PR.

[dnc77]

Ah, ok - no it's certainly not my intention to try and find a better way - my apologies I like this project and decided to have a look and since I'm still not 100% on the way git supports collaboration I simply did not realize work has already been done on this. I am familiar with the concept of a branch where changes are made and they get approved and merged back to the main trunk and I am still accustoming myself to pull requests, forks and how they work.

While I spent time on this, I could say that I familiarized myself a little with this project and since an icon already exists, the feature already exists and is being considered for merging to the main trunk, I would gather that no further effort is required on this - would that be correct?

Thanks

Duncan

[eishiya]

GitHub doesn't distinguish between Issues and PRs very clearly in the issue feed (the only difference is the PR icon in the status indicator, which is easy to miss), but you can see the PR linked right above your first post in this thread.

As far as I can tell from the PR thread, it's done, and no further work is required besides implementing the icon, which should just be a matter of including the file and setting the icon path. Perhaps comparing your code to the code in the PR may be educational though, to see if you missed something, or did something another way.

[dnc77]

Yes - thanks - I'll do that as a next step. I know what you mean also regarding the tool options which get loaded on a second toolbar when the tool is selected. I came across that functionality while I was working through this issue as well but what you mentioned solidified better my understanding in that area. I'll see how this was done to compare with my understanding. Thanks a lot!

[bjorn]

@dnc77 Indeed the change done in #3407 would be preferable, but thanks a lot for handling this problem anyway! If you find anything else you'd like to help improve, please don't hesitate to get in touch. :-)

Also, I think your approach to grouping the tools could be interesting, since some tools could benefit from this. For example, I think the "shape fill tool" would be nicer when separated into "rectangle fill" and "circle fill" tools that would be selected by such a dropdown, as opposed to having a toggle option for that after selecting the tool. Similarly, a "circle select" tool could be added grouped with the "rectangular select" tool.

[dnc77]

Hi @bjorn - thanks for following up with me.

I like how the project has been designed with AbstractTool and populateToolbar(). I came across that during my work but as I only just started looking at it; it did not occur to me that that was what was required.

Happy to help. I think the Shape Fill option might be worth visiting when there is more shapes; I'd be happy to split them up like you are saying if it proves better.

Re github: While I do have some experience and worked a lot with svn I'm also very cautious; I think the concepts remain very similar but I don't want to create something which does not fit the proper conventions of the repo with github. In that regard, I am a little unsure if one is to just take ownership of an issue and create a fork or work separately on it first. If one takes ownership, it may be that someone else can solve that issue faster given their knowledge. They will need to have write access to the fork. This is still an area in github I am starting to familiarize myself with but if there is somewhere I could follow in terms of repo guidelines and practices, I'd want to follow those and would be happy to read through them first!

Thanks again and great project!!