-
Notifications
You must be signed in to change notification settings - Fork 12
Migrate from PixiJS v6 to v8 #896
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
base: dev
Are you sure you want to change the base?
Conversation
- Add text utility to use `TaggedText`'s `textContainer` dimensions - Adjust Jest config to use pixi-tagged-text's CommonJS build, since their package doesn't use `type: "module"`
- 'pixi-tagged-text' requires `--legacy-peer-deps` for PixiJS v7 - It is still compatible for v7 - Can later be replaced by 'pixi-glyphs' in PixiJS v8 - Swap `interaction` boolean with `eventMode` enum (true = 'static', false = 'auto') - Set line height to 1 for the constraints' styled text - Change the RGBA hex number to string of same color (RGB hex is fine as is) - Import available filters from 'pixi.js' instead of individual packages - ColorMatrixFilter - FXAAFilter - BlurFilter - Import DropShadowFilter from 'pixi-filters' - Use `Ticker.targetFPMS` instead of `settings.Target_FPMS`
- Restores the ability to track the globalMouse position
- Set anchor to center for rotating texture renders - Render the wrapper Container to avoid clipping - Change reference from DisplayObject to Container - DisplayObjects are removed in Pixi.js v8 and recommended to use Container
- `pixi.js` re-exports the `@pixi/events` package - In PixiJS v8, the events sub-package will not exist
- Update logic to handle `Bounds` type returned from `display.getBounds()`
- Swap `DisplayObject` with `Container`
- Change `Container.name` to `Container.label` for debugger
- Add `DisplayUtil.isWorldVisible()` since it was removed from PixiJS v8
- Update `Text` constructor params to object
- Update `TextureUtil` to use `Assets.load()`
- Scale SVG resolutions by 2 on initial load, rather than on Sprite
creation
- Update `index.html.tmpl` to handle `app.run()` changing to async
- Update `Ticker.add()` signature to handle `Ticker` parameter
- Change `pixi.view` to `pixi.canvas`
- Add `LegacyPatchUtil` to handle modified native prototypes
- Prevent custom prototypes from breaking PixiJS initialization
- Update `Graphics` methods from deprecated to newer methods
- Add `fill()` and `stroke()` after paths are defined to draw the
graphics
- Swap `GradientFactory.createLinearGradient()` with `FillGradient` from
`pixi.js`
- Despite `@pixi-essentials/gradients` having an updated package for
PixiJS v8, it has an issue trying to make the base texture
- `Sprite` and `Graphics` calls to `addChild()` method has been deprecated - Replacing with `Container` where possible - Adding `Container` wrapper when replacement isn't possible - Replace `@override` JSDoc comments with TypeScript's `override` keyword - Change `HighlightBox` to extend `ContainerObject` instead of `GraphicsObject` - Change `ConstraintBar` to use `ContainerObject` with a `Graphics` added instead of using `GraphicsObject`
- Fixes changed behavior around getting content dimensions, since masks are included in the calculations
- Add patch-package to apply PixiJS patch
- Fix thumbnail from floating away by grouping the image goal background and the thumbnail into a container - Center the thumbnail on that container only when changing the thumbnail rather than on each resize event
- Removed extra `fill()` call on lock background
- Previous implementation had an extra `beginFill()` call that was ignored in
PixiJS <7 but was painting over the whole graphic in v8.
- Text dimension utility function was previously meant for pixi-tagged-text, but the dimension calculation was fixed for the pixi-glyphs package
- RankScroll was previously including the height of top/bottom RankBoards despite being masked but only on creation - On window resize the position is fixed - Call `onResize()` in MissionClearedPanel once on `lateUpdate` to fix the position
- Added `"type": "module"` to accurately reflect the compiled file type - patch-package needed the --exclude "^$" parameter to modify package.json
- Add `texture.source.update()` call for NGL sprite
- Check if borderColor and bgColor parameters are defined before drawing
- Updates `clone_object` to not use `array.clone()`
- Adjusts brightness to better match former visual
luxaritas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for all the work on this - really glad to see this (nontrivial) upgrade happen and some bonus cleanups to boot! And it looks like you've done a great job on QA.
Something that definitely needs to be looked at before merging is that it looks like screenshots are broken (see for example https://eternadev.org/sites/default/files/chat_screens/48290_1770939079.png).
One additional subtle issue is the specbox (hotkey S) now has a scrollbar when initially opened even though the content shouldnt be overflowing the window - I assume some measurement something has changed slightly, maybe text? If this isn't a quick fix it's so minor I'm happy for this to be just a filed issue for someone to address later.
| @@ -37,9 +37,9 @@ const measureMixin = { | ||
| const localBoundsCacheData = this._localBoundsCacheData; | ||
| localBoundsCacheData.index = 1; | ||
| localBoundsCacheData.didChange = false; | ||
| - if (localBoundsCacheData.data[0] !== this._didViewChangeTick) { | ||
| + if (localBoundsCacheData.data[0] !== ((this._didViewChangeTick & 0xffff) << 16) | (this._didContainerChangeTick & 0xffff)) { | ||
| localBoundsCacheData.didChange = true; | ||
| - localBoundsCacheData.data[0] = this._didViewChangeTick; | ||
| + localBoundsCacheData.data[0] = ((this._didViewChangeTick & 0xffff) << 16) | (this._didContainerChangeTick & 0xffff); | ||
| } | ||
| checkChildrenDidChange(this, localBoundsCacheData); | ||
| if (localBoundsCacheData.didChange) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per a comment I made in Slack, I believe we can remove this part of the patch since the issue was actually due to bounds mutation
| this._icon.removeChildren(); | ||
| this._icon.texture = Texture.EMPTY; | ||
|
|
||
| const iconYOffset = this._forMissionScreen ? 2.5 : 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, do you know why this is now necessary?
| fontSize: 11, | ||
| fill: 0xC0DCE7, | ||
| letterSpacing: -0.5, | ||
| align: 'center' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still desirable - see eg puzzle 13145953 on dev, where I've adjusted the min energy constraint to be multiline with two different line lengths
| resolution: devicePixelRatio | ||
| resolution: devicePixelRatio, | ||
| // Maintain renderer type from pixi.js before v8 | ||
| preference: 'webgl' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the rationale for this - isn't the default already webgl (and if the default changed to webgpu, wouldn't we want to allow that)?
| /** | ||
| * Returns the height/width of a text sprite. | ||
| */ | ||
| public static getTextDimensions(text: Text): {height:number, width: number} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why create a util for this as opposed to just calling getLocalBounds directly?
Summary
pixi.jsfrom v6 to v8.16 and related PixiJS dependenciespixi-filtersorpixi.jspixi-tagged-textwithpixi-tagged-text/pixi-glyphsTextwithtagStylesfrompixi.jsv8.16@pixi-essentials/gradientswithFillGradientfrompixi.jsGraphicsupdatedDisplayObjectwas removed entirely and replaced withContainerSpriteandGraphicscan no longer useaddChild()Containeror added a wrappingContainerto maintain layoutcontainer.getLocalBounds()does not take anyoutparameters anymore and returns a cachedBoundsinstancebounds.clone()inDisplayUtil.tsto avoid mutating the cachedBoundisWorldVisible()toDisplayUtilsince it was removed in PixiJS v8 with no replacementArray/Stringprototype mutations inutils.coffeeArraymethods added were enumerable and crashed initialization inpixi.jsv8Utils.clone_object()was updated to handle cloning arrays witharray.map()patch-packagefor modifyingpixi.js's behavior for returning stale bounds if modified in the same frame