Changed scale for cards to Vector2

[elmodor]

Cards used to have a Vector3 for the scale property. However, a card is scaled to 0.01 thickness by default. Increasing the thickness of cards leads to a weird collider behavior. A card is thin and if an object is thicker it should be of a different type (e.g. token). To reflect that the thickness of cards can not be changed and for easier configuration the scale was changed to Vector2.

This changes:

• Making the scale optionally a Vector2
• If scale is Vector2 apply a per-defined Y value based on type (for cards this would be 0.01 aka 1)
• Adjust the documentation, only using Vector2 for cards for now
• Forces scale Y (thickness) to be 1 for cards because a thicker cards leads to broken collider behavior

Fixes/Solves
Fixes #189

[drwhut]

Thank you for the PR! I just have a couple of thoughts about semantics:

• As well as Vector2, should cards also have the option to be defined by Vector3, even though the y component is not used at all? Maybe if a Vector3 is used, a warning or an error is thrown to signify that one should use a Vector2?
• On the flip side, should all other pieces have the option to have Vector2? I'm just worried this might cause potential confusion, as I imagine most people would want to make tokens thin. Plus, with the documentation stating that scale is a Vector3 for all objects, it feels wrong to let the user use another type entirely to define the property.

Thoughts?

[elmodor]

I have thought about those things as well. I was also not quite sure.

As well as Vector2, should cards also have the option to be defined by Vector3, even though the y component is not used at all? Maybe if a Vector3 is used, a warning or an error is thrown to signify that one should use a Vector2?

I tried setting the Y scale to 100 (which would basically be a 1) and the physics of the card were totally off. Unusable. So not sure what the benefit would be to make the card more thicker if your design was not meant for the card to be thick anyways. There's no gameplay benefit for the thickness. So that's why I chose not to allow it.
If you want we can allow it and fix the physics later (if possible?)
I just don't think we should let the user change a value that is not intended to be changed (for this specific object type)

On the flip side, should all other pieces have the option to have Vector2? I'm just worried this might cause potential confusion, as I imagine most people would want to make tokens thin. Plus, with the documentation stating that scale is a Vector3 for all objects, it feels wrong to let the user use another type entirely to define the property.

Why not? Default for all other objects is vector3 anyways. Should an error be thrown if someone declares another objects scale as vector2? (currently it would crash - or with the current PR just use the default Y scale).
Is it confusing? If a token is to thin/thick and someone uses vector2 I would assume it is very clear why. Also the documentation states vector3 for objects and vector2 for cards.
Should it say Objects (except cards)? Or keep it that way and allow cards to be vector3?

[drwhut]

I tried setting the Y scale to 100 (which would basically be a 1) and the physics of the card were totally off.

This is because the collision shape of cards is slightly thicker than their mesh, so increasing the y-scale also scales the difference in height.

If you want we can allow it and fix the physics later (if possible?)

Nah, I agree it shouldn't be changed (as you said, there is no benefit - one can make a token instead).

Should an error be thrown if someone declares another objects scale as vector2? (currently it would crash - or with the current PR just use the default Y scale).

In my personal opinion, an error should be thrown if a Vector2 is used for objects other than cards, as the y-scale is what I consider to be "necessary" information - plus, for tokens, one will almost always want to change the y-scale anyways.

Is it confusing? If a token is to thin/thick and someone uses vector2 I would assume it is very clear why. Also the documentation states vector3 for objects and vector2 for cards.

I'm not 100% convinced others will find that as intuitive as we do. I personally think the semantics I mentioned in the previous answer make the most sense (feel free to question it, though), but either way, the documentation needs to be as clear as possible as to avoid confusion.

Should it say Objects (except cards)? Or keep it that way and allow cards to be vector3?

Yeah, we want the documentation to be as specific as possible, so it's best to specify "Objects - Cards" use Vector3.

[elmodor]

So cards will use Vector2 and will force the Y scale to be 1.
I still want to allow Vector3 because all already created asset packs would break if we force a Vector 2 there? Not sure if that is good. A warning could be thrown but a user who uses an asset pack (not the creator) will have no idea what is wrong? Furthermore there is no harm really for allowing the vector3 for now.

I also adjusted the documentation to exclude Cards for vector3.

[drwhut]

That's a very good point - I think throwing a warning is the best way to go if a Vector3 is used for cards.

In terms of the errors and warning during importing being shown outside of the log files, I was planning to make them easily visible while working on #73.

[elmodor]

Sounds good. I added the warning

[drwhut]

Looks good, but I've got a couple of points that I think need addressing.

[elmodor]

I've split it up because you probably would not like:

scale = _get_file_config_value(config, from.get_file(), "scale", Vector3.ONE if type != "cards" else Vector2.ONE)

[drwhut]

Found a potential issue if an invalid type is used for objects that are not cards.

[GrimPixel]

The thicknesses of playing cards are usually between 0.17-0.24 cm, so if Y were 0.01, the shape and mass of cards would deviate from reality, which is avoidable. 0.02 is more realistic.

Following the question about “tiles” that leads here, the current solution looks like a split between cards and tiles. Anything with higher thickness will be placed in another category and currently, that category is called “token”. This means renaming “tokens” to “tiles”, I think. If so, then tiles need to behave identically in one's hand, otherwise a new category “tiles” will be required for them.

[elmodor]

Thinking about it, maybe we should not force a Vector2 for cards? If someone wants to modify the thickness, so let them? Any creator will try out their assets themself anyways and see if the physics still work for them or not?

[drwhut]

Maybe, but the problem is with the way cards are designed, they simply will not work as intended when the thickness is changed. The reason for the difference in mesh height vs collision shape height is because I want the mesh to be as thin as possible while having the physics engine being able to cope with the thin shape well enough. The ratio I found was the result of lots of tweaking and trial-and-error.

The thickness of cards could potentially be accounted for in the future - but for the 0.1.0 betas I do not want to change too much about the fundamentals of the game, since it could lead to more bugs / unexpected errors.

[elmodor]

I've incorporated your remarks. If it is not a card and the scale is not a vector3 it is set to the default value Vector3.ONE and an error should be thrown too.

[drwhut]

Looks good, will test before merging :)

[drwhut]

An error was thrown when trying to run the project:

[elmodor]

Sorry, should have verified that first myself. Should work now!

[drwhut]

All good now, thank you!