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

George 6.0.0 #3079

Closed
wants to merge 29 commits into from
Closed

George 6.0.0 #3079

wants to merge 29 commits into from

Conversation

Geokureli
Copy link
Member

Simply comparing the two branches for discussion with @EliteMasterEric. I have no intentions of merging this

Copy link
Member Author

@Geokureli Geokureli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comments...

@@ -609,6 +609,8 @@ class FlxCamera extends FlxBasic
itemToReturn = new FlxDrawItem();
}

if (graphic.isDestroyed) throw 'Attempted to queue an invalid FlxDrawItem, did you destroy a cached sprite?';

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this triggering for you? My hope with #2974 was to prevent these issues

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in discord:

FlxCamera Line 612 does trigger for me when I try to draw a sprite whose graphic has been disposed

@@ -511,6 +512,8 @@ class FlxGraphic implements IFlxDestroyable
*/
public inline function getFramesCollections(type:FlxFrameCollectionType):Array<Dynamic>
{
if (isDestroyed) return [];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question, should this have been added in #2974

@@ -116,6 +116,9 @@ class FlxDrawQuadsItem extends FlxDrawBaseItem<FlxDrawQuadsItem>
return;

var shader = shader != null ? shader : graphics.shader;

if (shader == null) throw 'Attempted to render an invalid FlxDrawItem, did you destroy a cached sprite?';

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, re: #2974

@@ -294,7 +294,7 @@ class BitmapFrontEnd
if (graphic != null)
{
removeKey(graphic.key);
graphic.destroy();
if (!graphic.isDestroyed) graphic.destroy();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, re: #2974

@@ -890,7 +890,7 @@ class FlxText extends FlxSprite
if (oldWidth != newWidth || oldHeight != newHeight)
{
// Need to generate a new buffer to store the text graphic
var key:String = FlxG.bitmap.getUniqueKey("text");
var key:String = FlxG.bitmap.getUniqueKey('text(${this.text})');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you talk about the issue this change solved, also, just made this #3081

@@ -62,7 +62,7 @@ class FlxGraphicsShader extends GraphicsShader
}
return vec4(0.0, 0.0, 0.0, 0.0);
}
")
", true)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this change?

@@ -499,7 +499,8 @@ class FlxGraphic implements IFlxDestroyable
if (collection.type != null)
{
var collections:Array<Dynamic> = getFramesCollections(collection.type);
collections.push(collection);
if (collections.indexOf(collection) == -1)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add this to flixel? what does it improve?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to say I made this change back when I was working on trying to resolve an issue with FlxAnimate. I'd definitely keep it as you don't want the list of collections to have duplicate references in it (as that can cause the reference counts to get messed up).

Comment on lines 132 to 141
public function pause():Void {
if (timer == null) return;
timer.active = false;
}

public function resume():Void {
if (timer == null) return;
timer.active = true;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty simple change, why not, same for preventing double release, below
Made an Issue: #3082

@Geokureli Geokureli marked this pull request as draft March 13, 2024 19:16
@Geokureli Geokureli closed this Jun 14, 2024
@Geokureli Geokureli deleted the george-6.0.0 branch June 14, 2024 16:27
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

Successfully merging this pull request may close these issues.

3 participants