SFML community forums

General => Feature requests => Topic started by: Foaly on July 17, 2013, 12:55:48 am

Title: Modulo for sf::Time
Post by: Foaly on July 17, 2013, 12:55:48 am
I wanted to do a modulo operation on a sf::Time object today and I was suprised there isn't one. Is there a specific reason for that? There is an overload for basicaly every other operation. Could it be added?
A use case would be something like this:
currentTime += deltaTime;
if(currentTime > duration)
{
    currentTime = sf::microseconds(currentTime.asMicroseconds() % duration.asMicroseconds());
}
would be shortend to this
currentTime = currentTime % duration;
// or even shorter
currentTime %= duration;
Title: Re: Modulo for sf::Time
Post by: Nexus on July 17, 2013, 01:08:49 am
Before you have modulo, division should be defined. That was discussed here (http://en.sfml-dev.org/forums/index.php?topic=6755.msg45015#msg45015).
Title: Re: Modulo for sf::Time
Post by: Laurent on July 17, 2013, 08:29:54 am
I don't see anything against this new operator, so yes it can be added in a future version.
Title: Re: Modulo for sf::Time
Post by: Foaly on July 17, 2013, 11:41:50 am
Great! That's good to hear. I guess since it won't be added in 2.1, because it is a new feature, I'll create a ticket on tracker so we don't forget about it.

I agree the / operator would be usefull too. It would shorten things like this:
float amount = currentTime.asSeconds() / duration.asSeconds();
// to this
float amount = currentTime / duration;

But there should be a notice in the documentation that if you want integer devision, you have to do something like this:
sf::Int64 amount = currentTime.asMicroseconds() / duration.asMicroseconds();
But I think for most cases a float will be what you want, to get a ration between 0 and 1 for interpolation or something alike.
Title: Re: Modulo for sf::Time
Post by: Nexus on July 17, 2013, 11:44:33 am
But I think for most cases a float will be what you want, to get a ration between 0 and 1 for interpolation or something alike.
That's also what I think. And it would be strange to have a modulo, but no division operator.

To get an integer, one could simply cast the result of the float division.
Title: Re: Modulo for sf::Time
Post by: Foaly on July 17, 2013, 11:50:07 am
To get an integer, one could simply cast the result of the float division.
Either that or if you don't want to loose percision use the code that I posted above. But I think it should definitly be marked in the documentation.

Here (https://github.com/SFML/SFML/issues/429) is the issue by the way.
Title: Re: Modulo for sf::Time
Post by: Laurent on July 17, 2013, 02:16:06 pm
Quote
And it would be strange to have a modulo, but no division operator.
Won't it be strange to have integer modulo and floating-point division anyway?
Title: Re: Modulo for sf::Time
Post by: Nexus on July 25, 2013, 02:00:53 pm
Won't it be strange to have integer modulo and floating-point division anyway?
The whole thing appears strange because different types are involved, which we are not used to.

But it makes sense when you look at the semantic definition of modulo, which uses the division. The physical dimensions ("scalar" denotes a dimensionless quantity) are also consistent.
time1 % time2 == time1 - floor(time1/time2) * time2
//   time     == time  -         scalar     * time

Furthermore, a division operator represents the inverse operation to the existing multiplication operator:
time1         == factor * time2
time1 / time2 == factor
//  scalar    == scalar
Title: Re: Modulo for sf::Time
Post by: pdinklag on July 26, 2013, 10:09:12 am
I'm really nobody to know C++ styles and standards, but in my view this is a case where the idea of operator overloading fails and named global functions should be used instead.

The "%" operator would be straightforward for integer modulo, but for any kind of division, "/" is too confusing, no matter which implementation you pick in the end. There are two good use cases for dividing times: one to calculate a ratio (floating point result), one to calculate the integer quotient or rounded down ratio (e.g. when turning seconds into minutes).

Both are equally common, I would guess, and therefore if SFML provides an operator for one case, it should provide one for the other case as well. Since you can't have "/" for both, maybe call them "Time::ratio" and "Time::quot" or something.

I'm not sure how painful this would be for C++ developers (I'm used to it coming from Java), so it's just my two cents on this.
Title: Re: Modulo for sf::Time
Post by: Nexus on July 27, 2013, 11:11:09 am
I'm really nobody to know C++ styles and standards, but in my view this is a case where the idea of operator overloading fails and named global functions should be used instead.
Generally, the rule of operator overloading is: Do it if it's intuitive ("as the ints do"). And the status quo is the counterpart: While
time1 == factor * time2
works, the inverse operation
time1 / time2 == factor
does not, although this relation is expected.

There are two good use cases for dividing times: one to calculate a ratio (floating point result), one to calculate the integer quotient or rounded down ratio (e.g. when turning seconds into minutes).
The second can be perfectly implemented using the first.

I'm not sure how painful this would be for C++ developers (I'm used to it coming from Java), so it's just my two cents on this.
As mentioned, it's unintuitive if some of the operations that exist for built-in types work, but others do not, although they belong to the same category of arithmetic operations.