HaxeFlixel / flixel

Free, cross-platform 2D game engine powered by Haxe and OpenFL
https://haxeflixel.com/
MIT License
1.97k stars 434 forks source link

Camera evidently limits collision logic in weird ways? #2082

Closed greysondn closed 7 years ago

greysondn commented 7 years ago

Report template? What's that? And I even suck at naming issues? Ah, geez. It gets better and better.

Here, have this:

C:\Users\greysondn>haxelib list
checkstyle: [2.1.12] 2.1.3
csv: [0.4.0]
dox: [1.1.0]
firetongue: [2.0.0]
flixel-addons: 2.1.0 2.2.0 2.3.0 [2.4.1]
flixel-demos: 2.3.0 [2.4.0]
flixel-retrotools: [0.1.1]
flixel-templates: 2.1.0 2.2.2 [2.4.1]
flixel-tools: 1.2.0 1.2.1 [1.3.0]
flixel-ui: 2.1.0 [2.2.0]
flixel: 4.1.1 4.2.0 [4.2.1]
format: [3.3.0]
hscript: [2.0.7]
hxcpp: 3.4.49 [3.4.64]
hxparse: [4.0.0]
kong.hx: 1.2.0 [1.2.1]
lime: [2.9.1] 3.7.4 5.0.3
openfl-compatibility: [1.0.1]
openfl-html5: [1.0.5]
openfl-native: [1.4.0]
openfl: [3.6.1] 4.8.1 5.1.2
task: [1.0.7]
yaml: 1.2.1 [1.3.0]

There's my environment sketch. Missing data points are that I am targeting - and have tested against -Flash. My player for debugging is standalone, Adobe Flash Debug Player 23. My IDE is the wonderful HaxeDevelop, v 5.1.1.1 . I've no interest to test against other targets, so consider them all question marks insofar as that goes.


Code snippet reproducing the issue:

package;

import flixel.FlxSprite;
import flixel.FlxState;
import flixel.group.FlxSpriteGroup;
import flixel.text.FlxText;
import flixel.util.FlxColor;
import flixel.FlxG;
/**
 * Demonstrate the current camera collision problem.
 * 
 * I've adapted this into a single class example from my current project.
 * Upstream is warmly encouraged to use this as a test and example case.
 * 
 * How hard can it possibly be?
 * 
 * @author Greysondn
 */
class CameraCollision extends FlxState
{
    // blobs that should always be inside
    public var innerBlobs:FlxSpriteGroup;

    // wall blobs, which should always be stationary relative to the world
    public var wallBlobs:FlxSpriteGroup;

    // A camera-holder blob, who fascinates the camera so much it chases
    public var cameraHolder:FlxSprite;

    override public function create():Void
    {
        // parent stuffs
        super.create();

        // create groups
        this.innerBlobs = new FlxSpriteGroup();
        this.wallBlobs = new FlxSpriteGroup();

        // setup the camera first and foremost
        this.cameraHolder = new FlxSprite();
        this.cameraHolder.makeGraphic(1, 1, FlxColor.TRANSPARENT);
        this.add(cameraHolder);
        this.cameraHolder.x = 320;
        this.cameraHolder.y = 240; // according to my notes, this is the middle of my camera's initial view
        this.cameraHolder.acceleration.y = 36;
        this.cameraHolder.maxVelocity.y =  35; // this way the camera starts from a standstill and goes faster.
        FlxG.camera.follow(this.cameraHolder); // hold the camera. Hold it!
        this.add(this.cameraHolder); // well baked. Add to state.

        // next we add the wallBlobs.
        for (_wallrow in 0...100) 
        {
            this.wallBlobs.add(createWallBlob( -32, _wallrow));
            this.wallBlobs.add(createWallBlob( 608, _wallrow));
        }
        this.add(this.wallBlobs);

        // next is the center blobs.
        for (_innerRow in 0...100)
        {
            this.innerBlobs.add(createInnerBlob(_innerRow));
        }
        this.add(this.innerBlobs);
    }

    public function createWallBlob(x:Int, row:Int):FlxSprite
    {
        // eventual return
        var _ret:FlxSprite = new FlxSprite(x);

        // vary the color so we have some sense of space
        if (1 == (row % 2))
        {
            _ret.makeGraphic(64, 64, FlxColor.BLUE);
        }
        else
        {
            _ret.makeGraphic(64, 64, FlxColor.CYAN);
        }

        // y coord is based on row
        _ret.y = row * 64;

        // they should be immovable
        _ret.immovable = true;

        // we're done
        return _ret;
    }

    public function createInnerBlob(row:Int):FlxSprite
    {
        // eventual return
        var _ret:FlxSprite = new FlxSprite(320, ((64 * row) + 32));
        _ret.makeGraphic(7, 7);

        // some basic physics here. this way it should give a healthy bounce and rest off the side walls
        _ret.elasticity = 0.45;
        _ret.mass = 50;
        _ret.drag.x = 36;
        _ret.drag.y = 36;

        // that's it.
        return _ret;
    }

    override public function update(elapsed:Float):Void
    {
        // parent things
        super.update(elapsed);

        // as they pass by the camera's point of view, hurl the inner blobs at the left wall
        for (blob in this.innerBlobs)
        {
            if (blob.y < this.cameraHolder.y && blob.ID != 42)
            {
                // set velocity
                blob.velocity.x = -354;

                // set id to arbitrary value so we only speed this once.
                // this is a quick and dirty solution to the problem of speed being set every frame.
                blob.ID = 42;
            }
        }

        // collide the two types of blobs
        FlxG.collide(this.innerBlobs, this.wallBlobs);
    }
}

Observed behavior: Beyond a certain point - I believe the world coordinates beyond where the camera is first focused - collision logic seems to break down. The example case provided shows that the pieces - the "inner blobs" as the code calls them - just hurl straight through the wall instead of stopping.

Expected behavior: In the provided example, we would expect that all the inner blob pieces bounce and come to rest roughly where they started.

starry-abyss commented 7 years ago

You should use FlxG.worldBounds.set(boundX, boundY, boundWidth, boundHeight) to set bounds of the collision area

greysondn commented 7 years ago

http://api.haxeflixel.com/flixel/FlxG.html#collide

Call this function to see if one FlxObject collides with another. Can be called with one object and one group, or two groups, or two objects, whatever floats your boat! For maximum performance try bundling a lot of objects together using a FlxGroup (or even bundling groups together!). This function just calls FlxG.overlap and presets the ProcessCallback parameter to FlxObject.separate. To create your own collision logic, write your own ProcessCallback and use FlxG.overlap to set it up. NOTE: does NOT take objects' scrollfactor into account, all overlaps are checked in world space.

There is no expected behavior here that should violate that premise.

In addition, it would appear that the system is expected to be intelligent enough that it doesn't require being handled with kid gloves for something so simple; for example, there is no such setting for the tutorial game: http://haxeflixel.com/documentation/zoom-and-cameras/

starry-abyss commented 7 years ago

I agree, this is a popular mistake, I made a PR for Cheat Sheet to include it, maybe worth to add in some other places too.

greysondn commented 7 years ago

I disagree about the mistake being mine; if the camera is following some given object, why doesn't it (at a minimum) set the collision bounds to follow itself? Further, why aren't they treated as "wherever objects span to" until the dev sets it more narrow for efficiency purposes?

There is a non-sane default being here unless there's some fundamental property or fact of the matter I missed. Considering it's badly documented enough to botch me up and I've been working with Flixel for about half a decade now (starting with the community branch for AS3), it's... not good.

MSGhero commented 7 years ago

Can you reduce the code to a minimal, barebones example that demonstrates your problem? Edit: never mind, I see what the problem is now.

JoeCreates commented 7 years ago

@greysondn The reason the worldbounds exists is because the collision system uses quadtrees to efficiently determine if a collision is taking place. Checking collisions across an infinite area would be very inefficient as you would be forced to check one object against every other, whereas the quadtree method allows you to eliminate many non-colliding objects at a time.

The reason the developer is expected to update these bounds themselves is because only the basic implementation is provided, and it makes no assumption about how a dev might want to use it. Any additional default behaviour would not only be extra code, it would also narrow the use cases, and many developers would find themselves having to override and remove the extra behaviour because it is not suitable for their purpose.

For example, it sounds like you are proposing that the default behaviour should be that the bounds automatically follow the camera. This would break cases where you need other objects in your world to be colliding still even when they are far away from the camera. (e.g. a large platformer)

Perhaps you might suggest the default behaviour to be automatically expanding the bounds, rather than moving it? This would inhibit the possibility of deliberately using a smaller worldbounds for efficiency, for games in which only nearby collisions matter. (e.g. an infinite runner)

The only sensible option is to have the developer decide how and when the worldbounds should be updated.

Gama11 commented 7 years ago

there is no such setting for the tutorial game

There is actually, there's an _mWalls.follow(); call on the previous page, which is a convenience method of FlxTilemap to adjust both camera and world bounds to the size of the tilemap.

I've thought about changing the default behavior of world bounds a while ago (#312), but eventually decided against it. Also, note that changing the default behavior of this would likely be pretty subtle breaking change.

Considering it's badly documented enough to botch me up and I've been working with Flixel for about half a decade now (starting with the community branch for AS3)

That's quite surprising to me, it has always behaved this way, even back in the AS3 days, see here for instance. There's a lot of mentions of this in the AS3 Flixel forums too, but those seem to be down right now.

I'm open to improving the documentation - the docs of FlxG.overlap() and FlxG.collide() should definitely mention FlxG.worldBounds.

greysondn commented 7 years ago

Yeah, I'm pretty known for causing Dovyski headaches back in AS3. I had rather planned not to cause them for HaXe-flixel and then this happens.... oh, nevermind.

Moving on.

Can you reduce the code to a minimal, barebones example that demonstrates your problem? Edit: never mind, I see what the problem is now.

I'd like to state for the record that it was verbosely commented and laid out this way so that compilation would make it self-evident and then the code could be examined. This shortens a few thousand lines of interactions to a practical demonstration that directly links to my own code in behavior. It appears, given the further comments, that this is a moot point, however.

The reason the worldbounds exists is because the collision system uses quadtrees to efficiently determine if a collision is taking place.

I'm aware. I've looked at re-implementing that for flood basin interactions in GIS systems, which is basically "I've even studied the underlying algorithm itself some" in my usual morose way.

Checking collisions across an infinite area would be very inefficient as you would be forced to check one object against every other, whereas the quadtree method allows you to eliminate many non-colliding objects at a time.

Is this meant to be follow-up on the nature of quad-tree or a counterpoint to my suggestion? In the former case, again, I'm aware; in the latter case, I wasn't suggesting it be used in an infinite area. The bounds could be where there are actually objects, and the spatial subdivisions offered by quadtree should, in theory, handle the messy details.

I'd be more concerned about "how many subdivisions" at that point. Which, always always always, quickly becomes the hairy "depends" programmers hate.

The reason the developer is expected to update these bounds themselves is because only the basic implementation is provided, and it makes no assumption about how a dev might want to use it.

...

Any additional default behaviour would not only be extra code, it would also narrow the use cases, and many developers would find themselves having to override and remove the extra behaviour because it is not suitable for their purpose.

That would require a back and forth impetus towards proving the point that I doubt either of us has time or interest in. Just because I'm aggressive pushing my points doesn't mean I'm interested in some pissing contest, which is what that'd quickly become.

For example, it sounds like you are proposing that the default behaviour should be that the bounds automatically follow the camera. This would break cases where you need other objects in your world to be colliding still even when they are far away from the camera. (e.g. a large platformer) ... Perhaps you might suggest the default behaviour to be automatically expanding the bounds, rather than moving it? This would inhibit the possibility of deliberately using a smaller worldbounds for efficiency, for games in which only nearby collisions matter. (e.g. an infinite runner)

So, to me, Flixel's heritage speaks to both platformers and infinite run games (eg; Mode and Canabalt). My suggestions were twofold; first that the bounds follow the camera by default (which does work in an infinite runner), and second that the bounds follow full scope of space where objects lay (such as a large platformer stage). Efficiency should never be premature. Defaults should be sane. To me, given the expectation that low-staff-count platformers is the most likely common use case for flixel, the bounds matching the extent of the space where objects lay is the sane default.

In other words, by trying to make no choice on what was best for developers, you did - in fact - create a situation where a choice may as well have been the cause. I can be honest enough to say that I don't actually have some exhaustive list of all the games ever made with Flixel; I am reasonable enough to guess that my presumptions on what the most likely use-case candidates are probably does at least fall somewhere in line with one of the larger segments, at least.

Late additional: If the developer wants efficiency on the premise of "oh no, my map is too large, I should only process part of it", that is a late-stage efficiency decision on the dev's shoulders, not an early stage one for the engine's. The engine should not be in the business of premature optimizations.

The only sensible option is to have the developer decide how and when the worldbounds should be updated.

I will repeat that this is one of those situations that merits quotes from Geddy Lee: "If you choose not to decide, you still have made a choice."

There is actually, there's an _mWalls.follow(); call on the previous page, which is a convenience method of FlxTilemap to adjust both camera and world bounds to the size of the tilemap.

I didn't bother to open this but I'll take it on faith. The buried reference would have missed me as I was searching for the specific function handle I was given earlier.

I've thought about changing the default behavior of world bounds a while ago (#312), but eventually decided against it. Also, note that changing the default behavior of this would likely be pretty subtle breaking change.

From the looks of it, the next major update is breaking changes. Such updates are good times. Under normal circumstances, I'd agree that this alone is enough reason to at least push such things back.

Looking over that issue (312), it appears that the approach isn't what I'd suggest. All (relevant) objects have data on their location and size; why not simply check if they're out of bounds (relative to worldbounds) and then expand it to fit them if they're not within it? The only problem that would arise I can foresee at first glance to that notion is objects that move unbounded. (Obviously having to adjust it every update for every object is unfeasible, but sooner or later even if a dev wanted infinite space to play in they'd have to make choices that, again, the engine shouldn't be in the habit of making.)

That's quite surprising to me, it has always behaved this way, even back in the AS3 days, see here for instance. There's a lot of mentions of this in the AS3 Flixel forums too, but those seem to be down right now.

Yeah, I dunno why the forums are always down. We'd have to go rounds about why I don't recall ever having this problem before; the code I wrote simply may not have needed me to worry, I may have forgotten, or the engine may be behaving differently in haxe. Or some other fourth thing.

I'm open to improving the documentation - the docs of FlxG.overlap() and FlxG.collide() should definitely mention FlxG.worldBounds.

At a bare minimum. Do update the docs with the caveat emptor; as I've pointed out before:

NOTE: does NOT take objects' scrollfactor into account, all overlaps are checked in world space.

Although after much talk I grasp it intends this in the context of scrollfactor, the semantics I gathered multiple times from reading it and checking my code against docs before this issue was opened was that overlaps can be trusted to be consistent relative to world space. Noting the caveat would be valuable and would probably head off many such issues in the future.

It's also within consideration to make some functions get forceful about collision checking - that is, "these will definitely, DEFINITELY check the collisions, regardless of worldbounds or any other usual concerns".

break for wandering eyes, general follow-through

Regardless of intended defaults blah blah blah - an argument that everyone will have for decades everywhere when it's not cut and dried - it's clear there's some issue in clarity here. Between the docs and Google, I didn't grasp this without filing a report. Please take me at my word that I was diligent and tried toying around with my own code before filing the report. Things shouldn't be like this for reasonable programmers. The barebones fix is to make it clearer that the worldbounds limit where collisions are actually checked; the documentation for those functions, as previously mentioned, is a pretty sane place for such notes as it's where it's most needed. Likewise somewhere relevant in the cheatsheet doesn't hurt - certainly, I keep a copy handy to brush rust out of my own gears when I get back to work.

I would recommend saner defaults beyond this. There is a behavior demonstrated; trying not to dictate behaviors to devs doesn't work here as there is still a behavior evident. Making sure the evidential behavior is reasonable should be on the list of things considered worthwhile. What is reasonable I can argue about all day but it is not my place alone to judge; I have offered my thoughts already on the matter.

Other things? Wrap up?

greysondn commented 7 years ago

L'esprit de l'escalier:

Is a utility function somewhere that asks all the objects in the stage's tree for their positions and sets worldbounds to have all of them within bounds within reason? It occurred to me a ways down the street after posting this. I am not suggesting that as a default behavior here, merely asking about feasibility for a utility function for what is probably a fairly common desired action.

Gama11 commented 7 years ago

It's also within consideration to make some functions get forceful about collision checking - that is, "these will definitely, DEFINITELY check the collisions, regardless of worldbounds or any other usual concerns".

The overlaps() functions of FlxSprite / FlxObject / FlxTilemap aren't based on quadtrees and as such don't have this bounds limitation, IIRC:

http://api.haxeflixel.com/flixel/FlxObject.html?#overlaps

greysondn commented 7 years ago

IIRC efficiency is down for that relative to FlxG. I presume this is actually the reason, however :/ .

Gama11 commented 7 years ago

Well, yes, it doesn't use quadtrees. :)

greysondn commented 7 years ago

For instant happiness, imagine how many idiots I must deal with on a daily basis to be used to writing exhaustive dissertations on simple internet comments. You don't get this morose and rational from being around an easy to work with team all the time. I always forget that upstream may disagree but isn't the same as my piece of downstream. I'm sorry for that portion of things.

Like I said, I had rather hoped I could avoid having anything complicated arise for haxe-flixel :/ I'm sincere in what I write but I don't mean to cause insanity. I'm not in any hurry at this point. I have the workaround/recommended solution from the community, the rest is just quibbling over what the thing to do here is.

Ohmnivore commented 7 years ago

Wow this was painful to read and needlessly overdrawn. You're lucky nobody took offense at your aggressive and accusatory tone.

greysondn commented 7 years ago

The fix is succinct and works.

needlessly overdrawn

I get about one chance a week to speak online. It is better for me to say too much and hope it all gets out. I have already apologized for the rest. I don't destroy records of what I've done; I am accountable for my actions, longer term. I have and will have nothing more to say on the matter.

moving along

That's not why I came back, however; I wanted to follow-up really quickly if someone else comes here through google or whatever.

FlxG.worldBounds.set(FlxG.camera.scroll.x, FlxG.camera.scroll.y, FlxG.camera.width, FlxG.camera.height)

If you have exactly one camera (the default, assumed here), this will make worldbounds mark to match the camera. Other people can help with further details on the topic.


Most of two years later (19 June 2019) I came back to this code with no memory of having gotten as far as I did, and found this error report. It's probably highly puzzling but my apology was issued in the discord channel. It is copied here for reference. The issue itself is long since resolved... I think. (Need to copy that into the new work on this project and see what happens, and it's not an issue with HaxeFlixel as the above will attest.)

image