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

Various FlxBar fixes #2938

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Various FlxBar fixes #2938

merged 1 commit into from
Oct 31, 2023

Conversation

Starmapo
Copy link
Contributor

  • Fixed frontFrames graphic not changing its useCount (for example, if you added an FlxBar and then called FlxG.bitmap.clearUnused(), the front graphic would be destroyed and the game would crash when trying to draw it)
  • Render-method-specific variables are now destroyed correctly. It used to try to destroy variables that are only used in the opposite render method
  • _frontFrame is now destroyed when being set to null
  • Fixed _frontFrame documentation referring to it as an FlxSprite
  • Removed redundant positionOffset = null code (the variable is already set to null earlier in the function)

- Fixed `frontFrames` graphic not registering as being used
- Render-method-specific variables are now destroyed correctly
- `_frontFrame` is now destroyed when being set to `null`
- Fixed `_frontFrame` documentation
- Removed redundant `positionOffset = null` code
@Geokureli
Copy link
Member

Can you create an example state that illustrates the issues in the old code that this PR fixes

@Starmapo
Copy link
Contributor Author

Can you create an example state that illustrates the issues in the old code that this PR fixes

Sure:

package;

import flixel.FlxG;
import flixel.FlxState;
import flixel.ui.FlxBar;

class PlayState extends FlxState
{
	var bar:FlxBar;

	override public function create()
	{
		bar = new FlxBar();
		bar.value = 50;
		bar.screenCenter();
		add(bar);

		super.create();
	}

	@:access(flixel.ui.FlxBar)
	override public function update(elapsed:Float)
	{
		if (bar != null)
		{
			if (FlxG.keys.justReleased.ONE)
			{
				// With the old code, the game will now crash when trying to draw the FlxBar,
				// as the front/filled graphic has been incorrectly cleared
				FlxG.bitmap.clearUnused();

				FlxG.log.add('Cleared unused graphics.');
			}
			else if (FlxG.keys.justReleased.TWO)
			{
				FlxG.log.add('Destroying the FlxBar...');

				final _frontFrame = bar._frontFrame;

				bar.destroy();

				// With the old code, these variables will not have been set to `null`
				if (FlxG.renderTile)
				{
					FlxG.log.add('frontFrames is null: ${bar.frontFrames == null}');
				}
				else
				{
					FlxG.log.add('_emptyBar is null: ${bar._emptyBar == null}');
				}

				// With the old code, _frontFrame will not have been destroyed
				FlxG.log.add('_frontFrame was destroyed: ${_frontFrame.sourceSize == null}');

				remove(bar, true);
				bar = null;
			}
		}

		super.update(elapsed);
	}
}

The last change (removed redundant positionOffset = null code) doesn't fix any bugs, just removes a redundant null set that already happens prior. (L199)

Comment on lines +201 to +203
if (FlxG.renderTile)
{
_frontFrame = null;
frontFrames = null;
Copy link
Member

Choose a reason for hiding this comment

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

Can you shed some light on this change? was renderBlit a mistake, or was the comment above wrong? why frontFrames instead of _frontFrames?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_frontFrame and _filledFlxRect are only used in tile render mode, but the current code mistakenly tries to destroy these when the game is using blit render mode, and vice versa for the variables only used in blit mode. Simply changing line 201 to renderTile fixes it.

I changed line 203 to frontFrames as that value is left without being set to null in the current code, and set_frontFrames will also null out _frontFrame.

@Geokureli Geokureli merged commit 38762cb into HaxeFlixel:dev Oct 31, 2023
16 checks passed
@Geokureli
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants