[object Object] instead of color for topColor bottomColor a-sky created from environment component

[vincentfretin]

The issue is in aframe 1.5.0, 1.6.0 and master.

Same when you copy the entity to clipboard:

It's because the environment component set directly a THREE.Color instance instead of hex color string on those properties, see
https://github.com/supermedium/aframe-environment-component/blob/b339b293139cd8c10f03d4c9a162c3b5ea55a794/index.js#L325-L326

@mrxz maybe you can take a look, that's probably a similar serialization issue than HTMLImageElement.

[mrxz]

The color property type has always been string based. Passing in a THREE.Color tends to work as components often pass it to Color.set which supports both. But on the A-Frame side there is no parse, equals or stringify method associated with it, hence the [object Object] when flushing to DOM.

Adding a specialized stringify that handles instance of THREE.Color would be a quick 'n dirty solution. A proper fix would be to have the color property either always parse to THREE.Color or explicitly handle either string or THREE.Color. Personally in favour of always parsing to THREE.Color, though that would be a breaking change for any component expecting a string value for color properties.

[vincentfretin]

Yeah that would be a breaking change for sure, it will break here for example
https://github.com/supermedium/aframe-environment-component/blob/b339b293139cd8c10f03d4c9a162c3b5ea55a794/index.js#L777
canvas fillStyle not expecting a THREE.Color instance.

Also in which color space would you create the THREE.Color? See for example usage here
https://github.com/supermedium/aframe-environment-component/blob/b339b293139cd8c10f03d4c9a162c3b5ea55a794/index.js#L325-L326

This change is a recent commit supermedium/aframe-environment-component@f972abe so we didn't have the [object Object] issue here before.

Can we just implement stringify for the color type to produce "#" + color.getHexString(); if it's a THREE.Color instance?

[mrxz]

Also in which color space would you create the THREE.Color?

It would apply the same logic as THREE does out of the box. So string based colours (CSS names, hex codes, etc...) are assumed to be sRGB and will be converted upon parsing into the working colour space. Components can then convert it to any colour space they might need, or leave it as-is when passing along.

When providing an instance of THREE.Color no conversion will take place (as neither .clone(), .copy(color) or .set(color) does). So this would give users/components the right level of control. Only when writing the HTML is the user "forced" to use sRGB exclusively, but this is in line with HTML/CSS standards.

While this might be a nice improvement over string-based colours, I don't think there's a way to introduce this without breaking some components.

Can we just implement stringify for the color type to produce "#" + color.getHexString(); if it's a THREE.Color instance?

Yes, though a more correct approach would actually be to implement a parse method that does this exact conversion. That way a component is guaranteed to receive a string-based colour, regardless if a string of THREE.Color instance was passed in. (So instead of parsing string -> THREE.Color, it parses THREE.Color -> string)