search

AgreeableDeerGames / GameBackbone

2D game framework
MIT License
11 stars 4 forks source link

C-Style casting is bad. And casting in general, should be the last option #184

Open lavinrp opened 5 years ago

lavinrp commented 5 years ago

In navigation demo we currently cast to float then back to int when we are creating our colors. This is a code smell. Something is probably wrong.

gyurgyma commented 5 years ago

In NavigationDemoRegion.cpp, the line of code is: sf::Uint8 red = (sf::Uint8)(255 * ((double)clusterNavigationWeights[i] / GB::BLOCKED_GRID_WEIGHT));

We can remove the casts by doing the multiplication first, followed by division. This effectively removes the need to have double math. sf::Uint8 red = (255 * clusterNavigationWeights[i]) / GB::BLOCKED_GRID_WEIGHT;

gyurgyma commented 5 years ago

RandGen::uniDist should handle "casting" to a template type. This will lead to undefined behavior when casting from float to int (loss of precision). This behavior should be defined in some way. One way is to define it using modulo, so that any number larger than INT_MAX no longer is: int i = f % INT_MAX This way is terrible and just another form of hack. I thought it was worth mentioning though as it lead to Ryan and me finding the solution we like. This solution is to utilize std::numeric_limits (http://www.cplusplus.com/reference/limits/numeric_limits/). We could check if our min/max are lessthan/greaterthan INT_MIN/INT_MAX and then either throw or something akin.