Fix FlxCamera "jitters" when following a target

[IQAndreas]

Resolves the following issue:

• FlxCamera follow jitters (FlxCamera follow jitters #5, FlxCamera follow jitters AdamAtomic/flixel#227)

Fix provided by Krix

[IQAndreas]

I have not tested the new code in a running project, but it compiles just fine without any error messages.

[Dovyski]

I really dislike the name simpleRender. It is meaningless in my opinion. The line:

((angle == 0) || (_bakedRotation > 0)) && (scale.x == 1) && (scale.y == 1) && (blend == null)

refers to the current sprite state (no rotation, no scaling, etc). For that reason, I think we should replace the get property simpleRender with a method onSimpleRender(). It is semantically consistent with similar methods such as onScreen().

[IQAndreas]

I don't quite follow.

The function onScreen() asks "Is the sprite on the screen?" The getter asks "Will/can this sprite be rendered in a simple fashion?" If so, the equivalent name would be willSimpleRender or isSimpleRender.

Otherwise, as far as I can tell, all functions that begin with on (such as onKeyPress or onMouseDown) are event handlers.

[Dovyski]

You are right. The name isSimpleRender is a good one. I think it fits better and will make the code easier to understand.

[IQAndreas]

Allrighty then, property renamed to isSimpleRender. Again, I haven't tested it, but it compiles without any errors.

I also added an additional check, making sure the target is a FlxSprite. That way if the target object is anything else, an error won't be thrown.

However, after checking the only other objects that extend FlxObject are FlxTilemap, FlxTile, and FlxQuadTree, and why anyone would follow a FlxTilemap around are beyond me. So, perhaps the error message is unnecessary? Or is it better to be safe than sorry?

[Dovyski]

Thanks! Your check is ok to me. It's better safe than sorry.

Just nitpicking, shoudn't we drop the get keyword in the isSimpleRender method? It is not getting any property, it is reporting some state. In my thoughts that's not a getter job.

[IQAndreas]

It is not getting any property, it is reporting some state.

Isn't a state a type of property?

I usually see functions as (1) actively "doing" something, or (2) a "dynamic getter" that returns different values based on the values passed in. Calling isSimpleRender several times in a row will keep returning the same value; it's only in special and more uncommon cases that it will return a different value. That is why I see it as a read-only property rather than a function.

Just because the function does more than just return _isSimpleRender; doesn't make it any less of a property to me. In fact, we could even populate an _isSimpleRender variable every time relevant settings change; this would avoid having to recalculate those values every frame (though, since all the checks are "is equal" or "is greater than", they aren't performance heavy at all).

[Dovyski]

I see your point. Most of the time I follow what you described, except when the very first idea related to the method is not related to getting a property.

As I said before, just nitpicking :)

[IQAndreas]

except when the very first idea related to the method is not related to getting a property.

I don't quite understand what you mean here. Could you clarify?

As I said before, just nitpicking :)

And I appreciate it. :)

Something very important to keep in mind in discussions with me: I can be very opinionated, but that doesn't mean I'm right. This goes for everyone, if you disagree with me, please tell me so.

[Dovyski]

"very opinionated" is good, it ensures we exchange ideas :)

What I meant about the "very first idea" is: if the method returns the value of a single property, then it should be a getter. If it returns the result of a test or a combination of property values (sum, etc), then it should be a non-getter method.

Try this straight analysis: dropping the use of methods, would it be possible to implement isSimpleRender using just a property? The answer is no, because it is a combination of other properties, it is a state, a result of a calculation, not a property.

[IQAndreas]

The answer is no, because it is a combination of other properties, it is a state, a result of a calculation, not a property.

Well, you could see numChildren and string.length as a result of a calculation rather than a property. But I still think they fit as read-only getters as the result is the same no matter how many times in a row you call the "function", and there are no "input variables" that change the outcome.

But I am adaptable. So, what's the final verdict: read-only getter or function?

[Dovyski]

haha, I guess we convinced each other :) Just nit-picking, numChildren and string.length are the result of a calculation, but both are not a combination of other properties. As I said, if you try dropping the use of methods, numChildren/ length would still work.

I would still go with a function.

[IQAndreas]

I would still go with a function.

Alrighty, done.

I also fixed some slight formatting so Dirk's changes match the rest of the code.

I haven't had a chance to test this, but I compiled the SWC and sent it to lib-dev in the Sandbox in case anyone would like to test if the changes worked.

[Dovyski]

I have a game with that jittering bug. I will test it against your SWC and check it everything is ok.

[WingEraser]

This fix cause a bug with multiple cameras that are used in Mode. It doesn't show correctly. Beside this bug, Mode doesn't even show the Menu and Emitter issue.

[WingEraser]

In FlxCamera::update() it should be deadzone = new FlxRect((width-w)/2,(height-h)/2,w,h);
However I notice the deadzone's x and y got an offset of 1. You need to do +1. This doesn't happen at flixel-gdx.

[Dovyski]

@WingEraser I've played Mode against the last dev version and the cameras seem to be working. What exactly was the problem you noticed?

[WingEraser]

See here: #179 (comment)