Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: small & stupid api changes: radians, vectors, colors  (Read 17518 times)

0 Members and 1 Guest are viewing this topic.

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
small & stupid api changes: radians, vectors, colors
« on: September 20, 2014, 12:12:19 am »
1.
setRadians and getRadians functions for angles in sf::Transformable. It would be backwards compatible, would not break the API, would let everyone use degrees OR radians, whichever they wish.
Yes, I know there is this thread, but it seems that it degraded into pointless argument about what is more 'friendly'...
http://en.sfml-dev.org/forums/index.php?topic=205.0
But yeah, sure - radians are less friendly and SFML is not hardcore-math-knowing-programmer's library. Only more reason for this change - it's very OK that 'default' unit is degrees and radians have to be explicitly present by name in functions that use them.
Yes, public API can do that but it's (literally) few lines of SFML code to save anyone working with radians from few lines of their own code, having to include the header with that code and then calling that single function differently than all other functions on all SFML transformables. It'll allow consistency in common user code. Radians are less common and friendly than angles but still common. This isn't as obscure and user specific as something like asking for allowing sin+cos for rotation would be.
It's a dumb change code wise, it won't break anything that now works with SFML and it'll make radians work out of the box.


2.
Small backwards compatible convenience change to sf::Vector2 constructor, instead of what we have now for casting from different SFML vector types (https://github.com/SFML/SFML/blob/master/include/SFML/System/Vector2.inl#L47) we could have this:
template <typename T>
template <typename U>
inline Vector2<T>::Vector2(const U& vector) :
x(static_cast<T>(vector.x)),
y(static_cast<T>(vector.y))
{
}
 
which just requires x and y fields in the class, not that it's SFML own class with another type, so vector from another library (say Box2D...) can be passed and work out of the box. This constructor is explicit anyway so it's pretty OK to do that IMO. It really makes sense that vector can be constructed out of something that has x and y in it (in particular - another vector with x and y in it).


3. Color should really have implicit constructor from sf::Uint32 (and possibly operator= too).
It's a bit annoying that I have to write that function myself and use it instead of just doing sf::Color(0xff0000ff) (or even passing number directly to function needing a color). Color and sf::Uint32 actually have same memory layout even, so you could memcpy from one to another so it's really funny there is no function to do that.
There is even implicit constructor from single UTF32 character to sf::String for convenience (which once bit someone in the ass) so why no that too?
HTML, GIMP, Ogre and probably many other use hex numbers like that already. Irrlicht has that kind of constructor so you can pass number to some of its' functions that need a color (some need float 0..1 colors, can't pass the number there).
It's really not at all obscure or unknown and it's really unlikely someone will pass a number instead of color accidentally and wonder what went wrong.
« Last Edit: September 20, 2014, 12:15:10 am by FRex »
Back to C++ gamedev with SFML in May 2023

Hapax

  • Hero Member
  • *****
  • Posts: 3379
  • My number of posts is shown in hexadecimal.
    • View Profile
    • Links
Re: small & stupid api changes: radians, vectors, colors
« Reply #1 on: September 20, 2014, 01:36:03 am »
I agree with all three of these with varying degrees (pun not intended) of strength.

1. If it's short, simple, doesn't break the API, expands its flexibility, and seems commonly wanted, I don't see why this wouldn't be included.

2. I've often found that I'm "converting" between a custom vector to an SFML one which requires manually assigning each member so I can see how this would be useful although I'm not good enough to know if this code solves everything without causing any problems.

3. This is strange because I've never even previously considered this but, now you mention it, it's something I've been missing for quite some time. Photoshop also uses hex numbers for colours; I thought I'd mention that one since you missed it off your list  ;) I must say, though, it would require still being able to assign the colours without the alpha part, and I'm not sure how that would be done here.
Selba Ward -SFML drawables
Cheese Map -Drawable Layered Tile Map
Kairos -Timing Library
Grambol
 *Hapaxia Links*

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 11034
    • View Profile
    • development blog
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #2 on: September 20, 2014, 01:53:47 am »
3. This is strange because I've never even previously considered this but, now you mention it, it's something I've been missing for quite some time. Photoshop also uses hex numbers for colours; I thought I'd mention that one since you missed it off your list  ;) I must say, though, it would require still being able to assign the colours without the alpha part, and I'm not sure how that would be done here.
Notice that FRex proposed an overload for sf::Uint32 which has 8 hex digits, 0xRRGGBBAA (R = Red, G = Green, B = Blue, A = Alpha).
I guess it could make things a tiny bit easier as oposed to using:
sf::Color c(0xff, 0x00, 0xee, 0xaa);
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

Hapax

  • Hero Member
  • *****
  • Posts: 3379
  • My number of posts is shown in hexadecimal.
    • View Profile
    • Links
Re: small & stupid api changes: radians, vectors, colors
« Reply #3 on: September 20, 2014, 01:58:10 am »
Notice that FRex proposed an overload for sf::Uint32 which has 8 hex digits, 0xRRGGBBAA (R = Red, G = Green, B = Blue, A = Alpha).
I guess it could make things a tiny bit easier as oposed to using:
sf::Color c(0xff, 0x00, 0xee, 0xaa);
I noticed it was for 8 hex digits, and you're right that it would make it easier to use than specifying the four components separately. However, it wouldn't allow to specify only the colour components.
That said, I'd be all for the four components in one number. I'd have no trouble getting into an automatic habit of typing an extra two fs for a standard (fully opaque) colour  :)
Selba Ward -SFML drawables
Cheese Map -Drawable Layered Tile Map
Kairos -Timing Library
Grambol
 *Hapaxia Links*

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #4 on: September 20, 2014, 10:30:05 am »
1. Bloated API. I don't think anybody will care about writing these two lines of code if they really need this feature
inline float deg(float rad) {return ...}
inline float rad(float deg) {return ...}

2. I haven't got any strong argument against it, but it feels like a hack, not a clean feature. It would only work for half of the other vector classes that exist in the wild, and may even produce hard to understand errors with some of them. Example: in Qt, vectors have x and y members but they are functions; I let you deduce what kind of error the compiler would output in this case.

3. Looks ok.
Laurent Gomila - SFML developer

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #5 on: September 20, 2014, 04:05:41 pm »
1. It's really minor bloat (if any) and sf::Transformable is a convenience class anyway.
It's still annoying to write your own code for something that common, it makes user code inconsistent with every other sf::Transformable use ever and many other things that you could write yourself as global functions using public sf::Transformable API (rotate, move, scale, Vector2f + two floats overloads of many functions) are in the class for convenience reasons already (I know Vector2f + two float overloads are supposed to go out but they somehow went in while they made less sense than this addition).
Right now we can do every common thing using built in methods except use radians.
You can't even rotate sprite to face mouse position without looking up a magic constant (either degrees per radian or PI, since M_PI is non standard) because atan2 returns radians. It's really annoying that a "simple" library can't do that out of the box.
« Last Edit: September 20, 2014, 04:07:46 pm by FRex »
Back to C++ gamedev with SFML in May 2023

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #6 on: September 20, 2014, 04:41:25 pm »
Quote
1. It's really minor bloat (if any) and sf::Transformable is a convenience class anyway.
It's not just sf::Transformable, it's everything that takes and returns angles.

Quote
I know Vector2f + two float overloads are supposed to go out
This is the direction we are taking. A lot of useless overloads were removed in SFML 2, and we'd rather remove others than add new ones.

Quote
You can't even rotate sprite to face mouse position without looking up a magic constant (either degrees per radian or PI, since M_PI is non standard) because atan2 returns radians. It's really annoying that a "simple" library can't do that out of the box.
Degrees to radians and the other way round are really common and simple operations, I've never been annoyed by them in any graphics program that I wrote with SFML.

If we ever add something to SFML, it would most likely be the two conversion functions that I posted previously. So it would be isolated and not require to add ugly overloads everytime we add a function that deals with angles.
Laurent Gomila - SFML developer

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #7 on: September 20, 2014, 05:55:20 pm »
There is something else that uses angles in SFML?
Vector shortcuts save you typing sf::Vector2f(), these ones save you a function call, looking up the constant online and writing the two functions (and then including them like crazy every single time you want to use a text, a sprite, a shape, etc.).
And there still are convenience functions like scale, move and rotate, do they go out too? They are even easier to write than degtorad and radtodeg.
Does everything go out except bare minimum? This is like heading towards "simple to read, simple to write, hard as f**k to use because you need to write 5 helpers for everything" direction. Simple doesn't have to mean poor and bare bones.

Quote
Degrees to radians and the other way round are really common and simple operations,
Now it's this, but before it often would be that "pi and radians are scary" and "degrees are more friendly". Seriously...

This has been mentioned times and times again by people:
http://en.sfml-dev.org/forums/index.php?topic=315.0
http://en.sfml-dev.org/forums/index.php?topic=205.0
http://en.sfml-dev.org/forums/index.php?topic=12146.0
http://en.sfml-dev.org/forums/index.php?topic=313.0
http://en.sfml-dev.org/forums/index.php?topic=14840.0
In 2008 you already said everyone is complaining about it missing.

This is really a wider problem (one of many) with SFML - mentality on new features: some things go in, some don't and there is no consistency about it other than someone having a gut feeling and/or eyeballing it. Up vector was pretty wonderful in that regard, few lines + oscillating between "I'll do it soon" and "we don't need it" for few years:
http://en.sfml-dev.org/forums/index.php?topic=2782.0
http://en.sfml-dev.org/forums/index.php?topic=6057.0
http://en.sfml-dev.org/forums/index.php?topic=11671.0
http://en.sfml-dev.org/forums/index.php?topic=14566.0

Concerning API bloat and DIY philosophy I honestly though I'd have 3. dismissed in record time by you. It's new implicit overload, it can be done by public API super easily (easier than radians) and it can be done with C++11 user literals easily too.
But somehow it's OK because other libraries, graphic programs and HTML, QML and so on do it too. ???
(Sadly that others-do-it reasoning doesn't work with adding radian helpers).
« Last Edit: September 20, 2014, 06:02:11 pm by FRex »
Back to C++ gamedev with SFML in May 2023

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: small & stupid api changes: radians, vectors, colors
« Reply #8 on: September 20, 2014, 06:31:24 pm »
1. Concerning get/setRadians, it would lead to a duplication of all angle-related methods in the API without adding new functionality. I don't consider this a good idea for the same reason we have no getSeconds/getMilliseconds/... in every place where time is used, or getX/Y for vectors, or getR/G/B/A for colors: combinatorial explosion. In my opinion, a more promising approach would be a dedicated type for angles, similar to sf::Time. Like that, type conversion and places where the type is used can be kept orthogonal.

2. Adding a template constructor is problematic because it accepts anything. If a type doesn't match, the compiler error will only appear in the constructor's implementation and not in overload resolution; it is usually more difficult to understand. I also think that providing support for only "x" and "y" is somewhat arbitrary... There's also no backwards conversion.

3. I wonder whether a constructor itself is the most expressive way... an alternative is a static function (named constructor idiom). What about reverse conversion (from colors to integers)?
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Hapax

  • Hero Member
  • *****
  • Posts: 3379
  • My number of posts is shown in hexadecimal.
    • View Profile
    • Links
Re: small & stupid api changes: radians, vectors, colors
« Reply #9 on: September 20, 2014, 07:55:08 pm »
dedicated type for angles[/url], similar to sf::Time
I agree with this if only to keep consistency within SFML. sf::Time can seem a little extra work sometimes but I like the forced type thing to help reduce errors.
As for not including degrees to radians functions because they're short and easy, think about the sf::Time functions. Divided by 1000? Thanks  :P

3. I wonder whether a constructor itself is the most expressive way... an alternative is a static function (named constructor idiom). What about reverse conversion (from colors to integers)?
redColor = sf::Color(#ff0000ff);
redInt = redColor.asInteger();
Possibles?
Selba Ward -SFML drawables
Cheese Map -Drawable Layered Tile Map
Kairos -Timing Library
Grambol
 *Hapaxia Links*

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #10 on: September 20, 2014, 09:20:25 pm »
Don't complain about my opinion. Now that SFML is driven by a team, it's just my own personal opinion and nothing more ;)

If everybody else is ok about 1. 2. and 3. then they will all be part of SFML, no matter what I say.

I just wanted to answer to something important in my opinion: there's a difference between adding a new feature to a class, and adding overloads related to a type everywhere. sf::Color, sf::String, ... I'd be happy to add new stuff to them, but I would strongly disagree to add overloads of every function that takes or returns them. That's why I like 3. but not 1.

And no, I don't like 3. just because other libraries do it. I thought you knew me better :P
« Last Edit: September 20, 2014, 09:24:29 pm by Laurent »
Laurent Gomila - SFML developer

Hiura

  • SFML Team
  • Hero Member
  • *****
  • Posts: 4321
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #11 on: September 21, 2014, 04:05:25 am »
Regarding 1, my opinion is the following. In order to avoid having a bloated API and possibly non consistent API the only solution I see is the addition of `sf::Angle` to represent angle regardless of radian or degree system. However, even if such class is relatively simple to implement we would probably provide a very minimalistic class (like we do with `sf::Vector2` for example) which would not be useful in practice to a lot of people since they would probably want an overloaded operator for their own type or a way to convert it to their own type of mathematical vector or simply because they already have such class in their code (this is from a personal experience). Instead I'd recommend keeping one type of angle and using primitive type as we currently do.

SFML / OS X developer

Foaly

  • Sr. Member
  • ****
  • Posts: 453
    • View Profile
Re: small & stupid api changes: radians, vectors, colors
« Reply #12 on: September 22, 2014, 03:06:40 pm »
Just wanted to say that I also think that 3 is a good idea!

cepro

  • Newbie
  • *
  • Posts: 12
    • View Profile
    • Email
Re: small & stupid api changes: radians, vectors, colors
« Reply #13 on: September 23, 2014, 02:09:05 am »
Indeed #3 would be a nice feature especially for people that are used to web development.

On another subject, it would also be great to add a setAlpha function to SFML graphic entities so one would avoid writing code like this :
sf::Uint8 alpha = 100;
const sf::Color& c = text.getColor();// text is an sf::Text
text.setColor(sf::Color(c.r, c.g, c.b, alpha));

Or maybe there is another straightforward way of writing this code that I'm not aware of?
« Last Edit: September 23, 2014, 02:10:37 am by cepro »

dabbertorres

  • Hero Member
  • *****
  • Posts: 505
    • View Profile
    • website/blog
Re: small & stupid api changes: radians, vectors, colors
« Reply #14 on: September 23, 2014, 03:04:42 am »
sf::Color newColor = text.getColor();
newColor.a = newAlpha;
text.setColor(newColor);

Looks a little cleaner to me.