mirror of
https://github.com/duckietm/Nitro-V3.git
synced 2026-06-19 15:06:20 +00:00
fix(layout-image): guard async image fetch with a request-id ref
LayoutFurniImageView and LayoutAvatarImageView both fired async image generation (TextureUtils.generateImage / SDK resetFigure callback) and wrote the result back through setImageElement / setAvatarUrl with only an isMounted / isDisposed component-level guard. If props changed twice in rapid succession the older request could resolve last and overwrite the newer image with a stale one, visible on slow connections or fast scroll over grids of unique items. Each effect now captures `const requestId = ++requestIdRef.current` and threads it into every async callback (TextureUtils.generateImage, the SDK's resetFigure listener, the cache write). When a callback fires it bails if `requestIdRef.current !== requestId` — only the latest effect's callbacks make it past the gate. A stale ENDED for the previous figure now leaves the cache and the rendered url unchanged. Moves both bugs from "Open" to "Recently fixed" in docs/ARCHITECTURE.md.
This commit is contained in:
+13
-15
@@ -776,24 +776,22 @@ this repo.
|
||||
|
||||
### Open
|
||||
|
||||
#### `LayoutFurniImageView` / `LayoutAvatarImageView` — async fetch race
|
||||
|
||||
In both files an effect kicks off an async `processAsImageUrl` /
|
||||
`generateImage` and writes the result via `setImageElement`. If props
|
||||
change twice in quick succession, the first fetch can resolve **after**
|
||||
the second one and overwrite the newer image with the older one.
|
||||
|
||||
**Fix shape**: capture a request-id ref at the start of the effect, only
|
||||
write the result if the ref hasn't been bumped meanwhile. Or — better —
|
||||
once React Query (#2) is enabled, model the image fetch as a query keyed
|
||||
on the props tuple; React Query handles cancellation and ordering for
|
||||
free.
|
||||
|
||||
**Severity**: visible only on slow connections / rapid prop changes. Not
|
||||
data-corrupting.
|
||||
_(none — both previously-listed bugs have landed; see "Recently fixed"
|
||||
below.)_
|
||||
|
||||
### Recently fixed (in this branch)
|
||||
|
||||
- **`LayoutFurniImageView` / `LayoutAvatarImageView` async fetch race
|
||||
fixed.** Both effects kicked off async image work
|
||||
(`TextureUtils.generateImage` / SDK `resetFigure` callback) and wrote
|
||||
the result via `setImageElement` / `setAvatarUrl` guarded only by an
|
||||
`isMounted` / `isDisposed` ref. If props changed twice in quick
|
||||
succession the older fetch could resolve last and overwrite the
|
||||
newer image. Both now capture a `requestIdRef` bumped at the start
|
||||
of the effect; the async callback bails when its captured id no
|
||||
longer matches the latest one. (React Query keyed on the props
|
||||
tuple would also work, but neither call goes through a composer /
|
||||
parser pair so the request-id ref is the lighter fix.)
|
||||
- **`MainView` CREATED/ENDED race fixed.** Two independent
|
||||
`useNitroEvent` listeners on `RoomSessionEvent.CREATED` /
|
||||
`RoomSessionEvent.ENDED` could land out of order under flaky
|
||||
|
||||
@@ -20,6 +20,12 @@ export const LayoutAvatarImageView: FC<LayoutAvatarImageViewProps> = props =>
|
||||
const [ avatarUrl, setAvatarUrl ] = useState<string>(null);
|
||||
const [ isReady, setIsReady ] = useState<boolean>(false);
|
||||
const isDisposed = useRef(false);
|
||||
// Request id bumped on every prop change. The SDK can call
|
||||
// resetFigure asynchronously when server-side figure data lands;
|
||||
// if props change in quick succession the older callback could
|
||||
// otherwise overwrite the newer image. The closure captures the
|
||||
// id and bails when stale.
|
||||
const requestIdRef = useRef(0);
|
||||
|
||||
const getClassNames = useMemo(() =>
|
||||
{
|
||||
@@ -52,6 +58,7 @@ export const LayoutAvatarImageView: FC<LayoutAvatarImageViewProps> = props =>
|
||||
{
|
||||
if(!isReady) return;
|
||||
|
||||
const requestId = ++requestIdRef.current;
|
||||
const figureKey = [ figure, gender, direction, headOnly ].join('-');
|
||||
|
||||
if(AVATAR_IMAGE_CACHE.has(figureKey))
|
||||
@@ -62,7 +69,7 @@ export const LayoutAvatarImageView: FC<LayoutAvatarImageViewProps> = props =>
|
||||
{
|
||||
const resetFigure = (_figure: string) =>
|
||||
{
|
||||
if(isDisposed.current) return;
|
||||
if(isDisposed.current || (requestIdRef.current !== requestId)) return;
|
||||
|
||||
const avatarImage = GetAvatarRenderManager().createAvatarImage(_figure, AvatarScaleType.LARGE, gender, { resetFigure: (figure: string) => resetFigure(figure), dispose: null, disposed: false });
|
||||
|
||||
@@ -74,7 +81,7 @@ export const LayoutAvatarImageView: FC<LayoutAvatarImageViewProps> = props =>
|
||||
|
||||
const imageUrl = avatarImage.processAsImageUrl(setType);
|
||||
|
||||
if(imageUrl && !isDisposed.current)
|
||||
if(imageUrl && !isDisposed.current && (requestIdRef.current === requestId))
|
||||
{
|
||||
if(!avatarImage.isPlaceholder())
|
||||
{
|
||||
|
||||
@@ -17,6 +17,11 @@ export const LayoutFurniImageView: FC<LayoutFurniImageViewProps> = props =>
|
||||
const { productType = 's', productClassId = -1, direction = 2, extraData = '', scale = 1, style = {}, ...rest } = props;
|
||||
const [ imageElement, setImageElement ] = useState<HTMLImageElement>(null);
|
||||
const isMounted = useRef(true);
|
||||
// Request id bumped by the effect on every prop change. The async
|
||||
// generateImage / imageReady callbacks capture it and only write
|
||||
// back if it still matches — prevents an older, slower fetch from
|
||||
// overwriting a newer one when props change in quick succession.
|
||||
const requestIdRef = useRef(0);
|
||||
|
||||
useEffect(() =>
|
||||
{
|
||||
@@ -28,13 +33,13 @@ export const LayoutFurniImageView: FC<LayoutFurniImageViewProps> = props =>
|
||||
};
|
||||
}, []);
|
||||
|
||||
const updateImage = useCallback(async (texture: any) =>
|
||||
const updateImage = useCallback(async (texture: any, requestId: number) =>
|
||||
{
|
||||
if(!texture) return;
|
||||
|
||||
const image = await TextureUtils.generateImage(texture);
|
||||
|
||||
if(image && isMounted.current) setImageElement(image);
|
||||
if(image && isMounted.current && (requestIdRef.current === requestId)) setImageElement(image);
|
||||
}, []);
|
||||
|
||||
const getStyle = useMemo(() =>
|
||||
@@ -62,12 +67,14 @@ export const LayoutFurniImageView: FC<LayoutFurniImageViewProps> = props =>
|
||||
|
||||
useEffect(() =>
|
||||
{
|
||||
const requestId = ++requestIdRef.current;
|
||||
|
||||
setImageElement(null);
|
||||
|
||||
let imageResult: ImageResult = null;
|
||||
|
||||
const listener: IGetImageListener = {
|
||||
imageReady: (result) => updateImage(result?.data),
|
||||
imageReady: (result) => updateImage(result?.data, requestId),
|
||||
imageFailed: null
|
||||
};
|
||||
|
||||
@@ -81,7 +88,7 @@ export const LayoutFurniImageView: FC<LayoutFurniImageViewProps> = props =>
|
||||
break;
|
||||
}
|
||||
|
||||
if(imageResult?.data) updateImage(imageResult.data);
|
||||
if(imageResult?.data) updateImage(imageResult.data, requestId);
|
||||
}, [ productType, productClassId, direction, extraData, updateImage ]);
|
||||
|
||||
return <Base classNames={ [ 'furni-image' ] } style={ getStyle } { ...rest } />;
|
||||
|
||||
Reference in New Issue
Block a user