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

Bug: Multiple instances of Component with Texture via useAssets #531

Open
itsezc opened this issue Aug 29, 2024 · 10 comments
Open

Bug: Multiple instances of Component with Texture via useAssets #531

itsezc opened this issue Aug 29, 2024 · 10 comments
Labels
bug Something isn't working v8 Issues related to Pixi React v8

Comments

@itsezc
Copy link

itsezc commented Aug 29, 2024

Current Behavior

When you have two Pixi components with a Sprite / Texture of the same URL, it doesn't show up when Mapped, I believe this is an issue with the underlying Cache.

Expected Behavior

N/A

Steps to Reproduce

N/A

Environment

  • @pixi/react version: v8 Beta
  • pixi.js version: v8
  • React version: 18.0.0
  • ReactDOM version: 18.0.0
  • Browser & Version: Chrome 127
  • OS & Version: MacOS 14

Possible Solution

No response

Additional Information

No response

@thejustinwalsh
Copy link

Would you mind adding a code snippet illustrating the issue for clarity?

@trezy trezy added bug Something isn't working v8 Issues related to Pixi React v8 labels Aug 29, 2024
@itsezc
Copy link
Author

itsezc commented Aug 29, 2024

As an example take the following React Component:

'use client';

import { type FC, useCallback } from 'react';

import { useAssets } from '@pixi/react';
import type { Container, Graphics } from 'pixi.js';

export namespace Character {
  export type Props = {
    name: string;
    config?: {
      x?: number;
      y?: number;
    };
  };
}

export const Character: FC<Character.Props> = ({
  name,
  config = { x: 0, y: 0 },
}) => {

  const {
    assets: [texture],
    isSuccess,
  } = useAssets([
    {
      alias: name,
      src: `/${name}.webp`
    },
  ]);

  const drawWrapper = useCallback(
    (graphics: Graphics) =>
      graphics
        .roundRect(0, 0, 40, 40, 6)
        .fill({ 0x95af94 }),
    [],
  );

  return (
    <container
      zIndex={2}
      x={config.x ?? 0}
      y={config.y ?? 0}
      scale={1}
      cursor='pointer'
      eventMode='static'
    >
      <graphics zIndex={1} draw={drawWrapper} />
      {isSuccess && (
        <sprite
          texture={texture}
          width={40}
          height={40}
          zIndex={2}
          anchor={0}
        />
      )}
    </container>
  );
};

When calling this Component with a map rendering multiple "versions" of this with the same name prop doesn't render the Sprite, but just the wrapper

@thejustinwalsh
Copy link

Thank you. Nice find!

I have yet to run into this specifically, as I was loading the asset in the parent component and passing the loaded texture into the child component I mapped over.

I can take a crack at adding a test case to https://github.com/pixijs/pixi-react/blob/beta/test/unit/hooks/useAssets.test.tsx. If you have any spare cycles and can capture the failure, it would greatly help expedite the fix. I can take over the failing PR.

I'm about to go on vacation and will be able to take a look at the end of next week.

@itsezc
Copy link
Author

itsezc commented Aug 29, 2024

Thank you. Nice find!

I have yet to run into this specifically, as I was loading the asset in the parent component and passing the loaded texture into the child component I mapped over.

I can take a crack at adding a test case to https://github.com/pixijs/pixi-react/blob/beta/test/unit/hooks/useAssets.test.tsx. If you have any spare cycles and can capture the failure, it would greatly help expedite the fix. I can take over the failing PR.

I'm about to go on vacation and will be able to take a look at the end of next week.

No issues, so when using the useAssets with this approach, what happens is the "first component" calls the useAssets hook, and renders, but for the "second component" it returns undefined. With some debugging I can see that the Pixi Cache is being set properly, but its being removed on an additional call of useAssets

@itsezc
Copy link
Author

itsezc commented Aug 29, 2024

For repro: https://codesandbox.io/p/sandbox/react-pixi-mapped-sprites-dk4jcn

@thejustinwalsh
Copy link

Thanks for the repro; I will probably have to mock fetch, as my current test and mocks do not appear to fail when I attempt to fail the test. Must mock deeper into the mock abyss.

thejustinwalsh added a commit to thejustinwalsh/pixi-react that referenced this issue Aug 30, 2024
thejustinwalsh added a commit to thejustinwalsh/pixi-react that referenced this issue Sep 1, 2024
@itsezc
Copy link
Author

itsezc commented Sep 2, 2024

Waiting on beta 13 🚀

@furic
Copy link

furic commented Sep 30, 2024

I'm with beta 14 and this issue seems still present.

I looked into the source code and the problem seems happen when allAssetsAreLoaded is true (on 2nd+ load and the asset is already presented in the cache), it does nothing and returns isSuccess false with assets empty.

const [state, setState] = useState<UseAssetsResult<T>>({
    assets: Array(assets.length).fill(undefined),
    isError: false,
    isPending: true,
    isSuccess: false,
    status: UseAssetsStatus.PENDING,
});

...

const allAssetsAreLoaded = assets.some(assetsLoadedTest<T>);

if (!allAssetsAreLoaded) {
   ...
}

return state;

Meanwhile, useAsset still works well although it's marked as deprecated:
Doesn't work:

const { assets: [texture], isSuccess } = useAssets(["assets/pipe-green.png"]);
return (isSuccess && <sprite texture={texture} width={10} height={50} />);

Works:

const texture = useAsset("assets/pipe-green.png");
return (<sprite texture={texture} width={10} height={50} />);

@thejustinwalsh
Copy link

thejustinwalsh commented Sep 30, 2024

@furic The PR that fixes this issue has not been merged yet and is still open #533

@rvion
Copy link

rvion commented Oct 20, 2024

I can confirm this issue is quite blocking for pretty common use-cases.
when you unmount any component that use a texture, and add it back later; it just doesn't display anymore

I'll just use the deprecated useAsset in the meantime :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v8 Issues related to Pixi React v8
Projects
None yet
Development

No branches or pull requests

6 participants
@thejustinwalsh @trezy @rvion @itsezc @furic and others