Updated the color setter so that it does not change the current alpha

[FriendlyGecko]

Will allow the closure of #1838.

I just swapped the last value self.color[3] with self._alpha which will the alpha getter to maintain the current.

[FriendlyGecko]

Recognized that that wasn't enough to solve the open issue so I modified the code to check for use of the arcade.Color type. Now it should be ready for testing/merge.

TO CLARIFY: I only modified the len = 3 check. Not len = 4, so users can still set the alpha through this if they wish.

[einarf]

We probably should have separate .rgb and .alpha properties and let the color one just take whatever length is passed in. This is more in line with arcade 2.6 and solves two issues.

[FriendlyGecko]

That makes sense, it is a little late for me now, but I will try to draft something like this when I get up.

[FriendlyGecko]

I created a workable draft, larger part will be cleaning up the RGB class which for the moment is just a copy of the Color class. As of right now, it is functional just not clean.

[pushfoo]

I know this is marked as a draft, but I'm not sure I understand all the changes intended or mentioned. I've commented on two specific places in the source.

larger part will be cleaning up the RGB class

einarf's comment above seems relevant here:

We probably should have separate .rgb and .alpha properties and let the color one just take whatever length is passed in.

I'm not yet sure having an RGB class is worth it at the moment. The main benefits I see are:

1. Cleaner RGB-only behavior
2. In theory, adding a arcade.color.rgb submodule which only has RGB colors

However, that has risks and costs:

1. Inheritance can slow things down, so copy and pasting code is a useful speed optimization
2. Copy and pasting creates riskjs of desyncing things which should have the same API behavior
3. Preventing that risk requires more complicated unit tests

These downsides are best dempnstrated by pyglet's shape, label, and sprite abstractions. The label and shape issues affect arcade since we depend on those classes. In C or C++, we could rely on the compiler to perform inlining, but Python doesn't have that option. For pyglet, we have to (eventually) rework tests to be better at enforcing API consistency in abstractions if we don't want to sacrifice execution speed.

[pushfoo]

This is cleaner, but it's worth doing benchmarks to see if it's slower. The reason pyglet code and the from_iterable method use the strange-looking unpacking method is that it's faster.

[FriendlyGecko]

"I'm not sure I understand all the changes intended or mentioned."

The intent is to make it so RGB and alpha are used together to create color and by default not changing the alpha value of a sprite unless specifically called on.

The idea being that RGB can more easily be manipulated separately.

[pushfoo]

Why remove this? My concerns:

• We don't yet have 3.12 compatibility merged (see Make arcade 2.x python 3.12 compatible #1953)
• We aim to support the lowest non-EOL version unless forced to drop it

[FriendlyGecko]

This wasn't intentional, I still have to clean it up before it is ready.

[pushfoo]

@FriendlyGecko Are you still interested in working on this?

TL;DR

1. An .rgb property with getter+setter is definitely good
2. We should defer an RGB class till a later PR.

How?

This is my opinion, but I'd take the following approach:

1. Make a backup branch with these changes and push it to your fork

2. Switch back to this PR's branch

3. Revert the following:

   • Changes to BasicSprite.color
   • The RGB class (We'll wait till later, see bottom of comment)

4. Add a Color.rgb property which returns self[:3]

5. Modify BasicSprite.rgb's setter to use an inlined style like Improved behavior of "visible" setter and getter properties in BasicSprite. #2029

6. Strongly consider reverting the _rgb slot

Why revert BasicSprite.color changes?

TL;DR: Computes rarely used data + has a merge conflict

We noticed visibility behavior is broken at the moment. There's already a PR to fix it over at #2029. The implementation there seems like good reference for a BasicSprite.rgb property since it seems to make efficiency improvements to the color setter. However, they also conflict with the current implementation of this branch.

Calculating an rgb value each time we set also doesn't seem worthwhile if:

• Color property writes are common
• Color property reads are rare
• The GPU is sent RGBA each time anyway

Why revert arcade.types.RGB for now?

TL;DR: Not a hard no forever, but we need to ship 3.0 + there are ongoing perf questions about tuple subclassing

New Features Discouraged until 3.1

To be clear, this isn't a hard no forever. However, the current priority is shipping Arcade 3.0. New features are somewhat discouraged for the moment unless:

• It's clear they won't break things somehow
• They need to be done now to prevent huge future pain

In this case, expanding the already-overfilled arcade.types module:

• risks breaking things
• doesn't seem necessary to prevent future issues

Issues with these are why #1772 is on hold.

Why revert the _rgb slot addition?

TL;DR: Duplicates rarely read data

If we read .rgb from sprites rarely, it's not worth:

• Duplicating data already found in BasicSprite.color
• Recomputing duplicated value with an expensive function call
• The complexity of a lazily updated property as an alternative

Perf Questions about Tuples / Etc

Another reason to wait is the perf profiling the pyglet project has been doing. They've been benchmarking variations on implementing vector types somewhat similar to Color.

Although their findings need further investigation, they suggest Arcade's colors might need a closer look. This applies to both the current Color and any future color types.

When do we add RGB as a type?

TL;DR: After we finish things critical to 3.0

Right now, the RGB class conflicts with the RGB type alias we have. There might be a way to change these aliases and their names to be nicer, but we first need to finish addressing a lot of other things.

1. Camera overhaul simple #1965
2. A big doc build system refactor to help with the camera PR
3. Various GL issues with blend state and texture managers
4. Various doc issues
5. Time / update issues
6. Input problems we should fix now
7. pyglet math type updates

I'm putting some thought into our nomenclature for colors again. There might be a way to use your concept. However, it might require cleaning up types. You can see that started in #2030, but nothing uses that alias yet, so it doesn't break anything. Avoiding major unintended breakage + the big todo list above both take priority.

[FriendlyGecko]

Marking as closed due to features being implemented in #2060