search

FlixelCommunity / flixel

Community fork of Adam “Atomic” Saltsman's popular game engine Flixel. Distilled from a variety of Flash games he worked on over the last couple years, including Gravity Hook, Fathom and Canabalt, its primary function is to provide some useful base classes that you can extend to make your own game objects.
http://flixelcommunity.org/
Other
84 stars 17 forks source link

Fix a regression with FlxSprite's _maxFrames calculation. #181

Closed Dovyski closed 11 years ago

Dovyski commented 11 years ago

The current approach assumed perfectly filled spritesheets rows. It used ceil() to adjust any floating value, which resulted in a blank column at the right of some graphics.

I've noticed that testing with a game that uses particles. Some particles had that blank column. Removing the ceil (which was Adam's original approach) fixed the problem.

IQAndreas commented 11 years ago

We discussed this one before:

The problem with not using any sort of rounding is that the value of maxFrames will be completely off.

Say, if your sprites are 10px each, the sprite sheet 45px wide and 40px tall, using Math.ceil() with maxFrames will say there are 18 frames, when really there can only be either 16 or 20 frames depending on if you count the "overhang" as a valid frame or not.

Personally, I would prefer using Math.floor() and just completely ignore any overhanging pixels. I also mentioned outputting a warning if this was the case, but forgot to implement that.

Dovyski commented 11 years ago

I remember our discussion. For some reason I had in my mind the idea that we used Math.ceil() as a test. If everything worked, we would keep it, otherwise it would be changed.

Using Math.floor() is the same as my current fix: I don't floor() the results, but they will be automatically casted to uint during the variable assignment.

I think we should merge this pull request, since it will prevent lots of "blank" or "half-blank" frames all over the place after people start using Flixel Community (my game included).

IQAndreas commented 11 years ago

they will be automatically casted to uint during the variable assignment.

Oh, right. Yes, that would do the trick.

Although, would it be possible to explicitly run it through FlxU.floor() just to make it clear that those extra pixels are actually being trimmed off? It shouldn't affect performance as _maxFrames is only re-calculated when the the sprite sheet is set.

Do you want to add that warning if the sprite sheet is a few pixels too large as well, or push it up to a future version?

Dovyski commented 11 years ago

I've just added the FlxU.floor() to make the trimming explicit. About the warning, I think we shoud push it up to the future version.

IQAndreas commented 11 years ago

The code looks flawless (there's not much one can botch by accident in two lines ;) ).

Could test this in one of your games, and if it fixes the problem, go ahead and merge this?

Dovyski commented 11 years ago

haha, indeed there's not much to mess up in two lines :) I've tested and everything looks good.