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

Remove unused legacy code & defines #3059

Open
wants to merge 6 commits into
base: release6
Choose a base branch
from

Conversation

Starmapo
Copy link
Contributor

@Starmapo Starmapo commented Mar 5, 2024

This PR removes a lot of code that goes unused in current versions of HaxeFlixel.

  • Removed the FLX_POST_PROCESS define & its related code. This was only available in versions of OpenFL before 4.0.0, which is not supported in the latest HaxeFlixel (minimum OpenFL version is 9.2.2).
  • Removed references to the openfl_next, lime_legacy & next defines. These defines were removed in the same commit as openfl_legacy, which was also already removed from HaxeFlixel in Removing all references of openfl_legacy. #2990.
  • Removed Lime & OpenFL version checks that are always true/false due to the current minimum versions (for example, #if (lime >= 7.0.0) is always true due to the minimum version being 8.0.2).

There are a few breaking changes:

  • The PostProcess class was deleted, which was still available when FLX_POST_PROCESS wasn't defined, though it was merely a placeholder. addPostProcess and removePostProcess were also removed from FlxG, having the same effect.
  • dump and undump have been removed from FlxGraphic, due to dump only doing anything when lime_legacy was defined. Due to this, all variables and functions that made use of these were removed, including FlxG.bitmap.onAssetsReload which refreshed the assets by calling undump whenever a change event was dispatched in openfl.utils.Assets. However, I could not find a single instance in Lime or OpenFL when this event would actually be dispatched, so it doesn't seem to have any purpose unless the user is meant to dispatch it themselves.

Starmapo added 2 commits March 5, 2024 17:22
- Removed `FLX_POST_PROCESS` define &  related code
- Removed references to `openfl_next`, `lime_legacy` &  `next` defines
- Removed Lime & OpenFL version checks that are always true/false due to the current minimum versions
@Geokureli Geokureli changed the base branch from dev to release6 March 5, 2024 22:32
@Geokureli
Copy link
Member

Geokureli commented Mar 5, 2024

Because of the breaking changes, I've changed this PR so it will go to the 6.0.0 release branch rather than dev. @Starmapo please, backmerge the release6 branch into this branch, just in case

CI Errors:

TexturePackerAtlas Demo

source/MenuState.hx:128: characters 15-24 : flixel.system.frontEnds.BitmapFrontEnd has no field dumpCache

PostProcess Demo

source/PlayState.hx:6: characters 8-46 : Type not found : flixel.effects.postprocess.PostProcess
source/PlayState.hx:16: characters 28-39 : Type not found : PostProcess
source/PlayState.hx:16: characters 28-39 : ... For function argument 'child'
source/Main.hx:6: lines 6-13 : ... Defined in this class

@Starmapo
Copy link
Contributor Author

Starmapo commented Mar 5, 2024

Huh, I wasn't expecting there to be demos that use these features. TexturePackerAtlas was added 11 years ago, and PostProcess isn't even on the site currently. I'll make a PR over there fixing it.

Merged the release6 branch like you said 👍

@Geokureli
Copy link
Member

@EliteMasterEric was requesting expansion of the FlxGraphic.dump feature, here: #3034

this PR seems to directly conflict with that

@Starmapo
Copy link
Contributor Author

Starmapo commented Mar 6, 2024

@EliteMasterEric was requesting expansion of the FlxGraphic.dump feature, here: #3034

this PR seems to directly conflict with that

I was originally going to make a PR for that, but then noticed that there were already FlxGraphic.dump and undump functions. There isn't a way to use them in the current version of HaxeFlixel due to the dumping functionality being enclosed in a lime_legacy define, so this PR removes them, and we'd just have to re-add the functions whenever we add the new dumping method.

If you do see a reason to keep them in this PR, then let me know.

Geokureli added a commit to Geokureli/flixel that referenced this pull request Mar 6, 2024
flixel/FlxGame.hx Outdated Show resolved Hide resolved
I mistakenly removed the entire code due to it including `lime_legacy`.
@Geokureli
Copy link
Member

what is the plan, now?

@Starmapo
Copy link
Contributor Author

We have to keep FlxG.bitmap.onAssetsReload which in turn means we have to keep the undump function which refreshes the bitmap data, regardless of if it was dumped before. However since dump is getting removed I asked you if it should be renamed to something more appropiate like refreshBitmap, or if the code should just be kept inside onAssetsReload.

@Geokureli
Copy link
Member

I asked you if it should be renamed to something more appropriate like refreshBitmap, or if the code should just be kept inside onAssetsReload.

Sorry, I missed that, yeah I think thats a good idea

…Refreshed` respectively (old ones are now deprecated)

- Also brought back `onAssetsReload()` & `getBitmapFromSystem()`
@Starmapo
Copy link
Contributor Author

I decided to bring back undump() and canBeDumped renamed to refreshBitmap() and canBeRefreshed respectively, as well as the old ones but marked as deprecated, since it's possible that someone was using them for the refreshing function alone. Do let me know if you have any more suggestions.

charlesisfeline pushed a commit to VsFreyaDevs/flixel that referenced this pull request Aug 19, 2024
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