Skip to content
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

don't enable the colour tickboxes by default #57

Merged
merged 2 commits into from Jul 1, 2018

Conversation

JohnFlowerful
Copy link
Contributor

No description provided.

@qaisjp
Copy link
Contributor

qaisjp commented Oct 16, 2016

Why is this change necessary?

@JohnFlowerful
Copy link
Contributor Author

It's annoying having all colours of a vehicle change when colour picker is opened for the first time. Red isn't always the prettiest colour~

@Dutchman101
Copy link
Member

I don't agree. Players are used to the color changing as they use slider in the color mixer, now it won't be a realtime effect unless they tick it first - many players will not understand it they need to tick boxes.
It's extra effort required where it aint really needed. No from me

@qaisjp
Copy link
Contributor

qaisjp commented Oct 17, 2016

Why don't we default it to the current settings?

@JohnFlowerful
Copy link
Contributor Author

@Dutchman101 I dunno if players will find unticked checkboxes harder than ticked checkboxes, but I'm sure most will be able to figure it out.
wrt it being needed: a lot of custom vehicle and rim models utilise all of the colour 'slots' available.

@qaisjp it's not that easy I'm afraid. The colour picker only takes one colour as an input. Switching between which 'slot' (i.e. secondary vehicle colour, headlights etc.) it's applied to is controlled by the checkboxes.

@qaisjp
Copy link
Contributor

qaisjp commented Oct 17, 2016

Why not make the picker default to primary and the existing vehicle colour?

@Dutchman101
Copy link
Member

Dutchman101 commented Oct 17, 2016

@JohnFlowerful I didn't mean to say many players will find it hard, it can be a very low amount of them but still, this change doesnt outweigh taking that for granted; I made my comment more in the context of ''why complicate things when not really needed''.
That means I dont agree on your argument its botherful that it defaults to Red color, because a player who starts setting a color is likely to continue picking his color that he wishes to set. If you see alot of red cars driving around (based on your opinion, bad) that means they just wanted to get rid of an old color and found red fine already.

Your opinion about red therefore is a trivial thing, and I dont see a reason to change (as I said, why change/complicate things for no reason but your own trivial opinion about a default color: people using color function, obviously will pick their custom color if they wish, if they dont, then just leaving it to be red is their choice, not something we should structurally prevent just beccause you* dont like it)

@Dutchman101
Copy link
Member

You can also put it in this way: it's convenient to let it automatically set colors because it requires less steps to start setting color by the colorslider, does the (your) reason outweigh increasing the amount of steps needed to get going? (the need to tick checkboxes)

I think in that comparisation, the trivial reason for this change (in my opinion) doesn't justify more hassle for the player. Even if it's not much hassle, if there's no proper need, why?

@JohnFlowerful
Copy link
Contributor Author

@qaisjp that's a sensible idea~

@Dutchman101 updated the pr to include both your and qaisjp's suggestions :)

@Dutchman101
Copy link
Member

Dutchman101 commented Oct 17, 2016

@JohnFlowerful nah, my objections focus on the unticked state of the checkboxes. Please then remove those commits from the PR that make the change I find unnecesary; leave the tickboxes state on default ticked.

Now that you no longer have your ''red color'' concern as reason, what would be a new reason to set them unticked by default, if you don't exclude that?

@JohnFlowerful
Copy link
Contributor Author

@Dutchman101 the red is not the issue. It's how colour picker is setting all colours of a vehicle when opening for the first time.

I've updated the pr again to not be silly. This should satisfy both of our needs.

@Dutchman101
Copy link
Member

You're still doing the same thing.. let's wait and see if it goes through with other opinions. I stay with my arguments

@JohnFlowerful
Copy link
Contributor Author

@Dutchman101 colour 1 is still ticked by default. That is what you wanted right?

@Dutchman101
Copy link
Member

@JohnFlowerful
Many vehicles work with more than 1 color, not setting all will result in part of body being colored only.
Besides that, not all vehicles are set to colour the main body through color 1. Primary colors can differ.

@ArranTuna
Copy link
Contributor

"It's annoying having all colours of a vehicle change when colour picker is opened for the first time." - Your code fixes that at least so could just add this part of the patch.

@qaisjp qaisjp merged commit 0577fc1 into multitheftauto:master Jul 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants