Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using group.length after removing an element yields a null reference due to not updating the internal value #2010

Closed
Jarrio opened this issue Dec 6, 2016 · 5 comments

Comments

@Jarrio
Copy link

Jarrio commented Dec 6, 2016

  • Flixel version: 4.2.0
  • OpenFL version: 3.6.1
  • Lime version: 2.9.1
  • Affected targets: All of them

Code snippet reproducing the issue:

import flixel.FlxSprite;
import flixel.FlxG;
import flixel.FlxState;
import flixel.group.FlxSpriteGroup;

class TestState extends FlxState {
    public var group:FlxSpriteGroup = new FlxSpriteGroup();
    override public function create():Void {
        super.create();

        for (i in 0...10) {
            group.add(new FlxSprite(i * 10, 10));            
        }

        add(group);
        group.remove(group.members[0], true);

        
        for (i in 0...group.length) {
            trace(group.members[i].x);
        }
        
    }
}

Observed behavior:
Error is on the final trace
Expected behavior:
The group.length value should match the new group.members.length value
Temporary fix is to just use the group.members.length value

@Jarrio Jarrio changed the title Using group.length after removing an element yield a null reference due to not updating the internal value Using group.length after removing an element yields a null reference due to not updating the internal value Dec 6, 2016
@Gama11
Copy link
Member

Gama11 commented Dec 6, 2016

Nice catch. Looks like there originally used to be a length-- there, but it went missing between these two commits:

2ddde96#diff-0a33dc2c18b9746c9928ecafde442aa2L344
e353f07#diff-0a33dc2c18b9746c9928ecafde442aa2R343

@Gama11 Gama11 added the Bug label Dec 6, 2016
@Gama11 Gama11 closed this as completed in 1703068 Dec 6, 2016
@MSGhero
Copy link
Member

MSGhero commented Dec 7, 2016

#1887 though?

@Gama11
Copy link
Member

Gama11 commented Dec 7, 2016

That's not really the same thing. That PR would've made it so that length is always decreased in remove(), even with Splice = false.

@MSGhero
Copy link
Member

MSGhero commented Dec 7, 2016

I meant more that you said you were wary of changing it bc people relied on the current behavior, and it's a breaking change.

But it's more correct now so...

@Gama11
Copy link
Member

Gama11 commented Dec 7, 2016

Ah. I mean, this really just restores the original / "correct" behavior. We couldn't fix any bugs if we always fear somebody relies on the buggy behavior. :)

That PR you linked went in a different direction / changed the behavior, it wasn't a bugfix.

Aurel300 pushed a commit to larsiusprime/haxeflixel that referenced this issue Apr 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants