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
|
### Open
|
||||||
|
|
||||||
#### `LayoutFurniImageView` / `LayoutAvatarImageView` — async fetch race
|
_(none — both previously-listed bugs have landed; see "Recently fixed"
|
||||||
|
below.)_
|
||||||
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.
|
|
||||||
|
|
||||||
### Recently fixed (in this branch)
|
### 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
|
- **`MainView` CREATED/ENDED race fixed.** Two independent
|
||||||
`useNitroEvent` listeners on `RoomSessionEvent.CREATED` /
|
`useNitroEvent` listeners on `RoomSessionEvent.CREATED` /
|
||||||
`RoomSessionEvent.ENDED` could land out of order under flaky
|
`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 [ avatarUrl, setAvatarUrl ] = useState<string>(null);
|
||||||
const [ isReady, setIsReady ] = useState<boolean>(false);
|
const [ isReady, setIsReady ] = useState<boolean>(false);
|
||||||
const isDisposed = useRef(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(() =>
|
const getClassNames = useMemo(() =>
|
||||||
{
|
{
|
||||||
@@ -52,6 +58,7 @@ export const LayoutAvatarImageView: FC<LayoutAvatarImageViewProps> = props =>
|
|||||||
{
|
{
|
||||||
if(!isReady) return;
|
if(!isReady) return;
|
||||||
|
|
||||||
|
const requestId = ++requestIdRef.current;
|
||||||
const figureKey = [ figure, gender, direction, headOnly ].join('-');
|
const figureKey = [ figure, gender, direction, headOnly ].join('-');
|
||||||
|
|
||||||
if(AVATAR_IMAGE_CACHE.has(figureKey))
|
if(AVATAR_IMAGE_CACHE.has(figureKey))
|
||||||
@@ -62,7 +69,7 @@ export const LayoutAvatarImageView: FC<LayoutAvatarImageViewProps> = props =>
|
|||||||
{
|
{
|
||||||
const resetFigure = (_figure: string) =>
|
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 });
|
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);
|
const imageUrl = avatarImage.processAsImageUrl(setType);
|
||||||
|
|
||||||
if(imageUrl && !isDisposed.current)
|
if(imageUrl && !isDisposed.current && (requestIdRef.current === requestId))
|
||||||
{
|
{
|
||||||
if(!avatarImage.isPlaceholder())
|
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 { productType = 's', productClassId = -1, direction = 2, extraData = '', scale = 1, style = {}, ...rest } = props;
|
||||||
const [ imageElement, setImageElement ] = useState<HTMLImageElement>(null);
|
const [ imageElement, setImageElement ] = useState<HTMLImageElement>(null);
|
||||||
const isMounted = useRef(true);
|
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(() =>
|
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;
|
if(!texture) return;
|
||||||
|
|
||||||
const image = await TextureUtils.generateImage(texture);
|
const image = await TextureUtils.generateImage(texture);
|
||||||
|
|
||||||
if(image && isMounted.current) setImageElement(image);
|
if(image && isMounted.current && (requestIdRef.current === requestId)) setImageElement(image);
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
const getStyle = useMemo(() =>
|
const getStyle = useMemo(() =>
|
||||||
@@ -62,12 +67,14 @@ export const LayoutFurniImageView: FC<LayoutFurniImageViewProps> = props =>
|
|||||||
|
|
||||||
useEffect(() =>
|
useEffect(() =>
|
||||||
{
|
{
|
||||||
|
const requestId = ++requestIdRef.current;
|
||||||
|
|
||||||
setImageElement(null);
|
setImageElement(null);
|
||||||
|
|
||||||
let imageResult: ImageResult = null;
|
let imageResult: ImageResult = null;
|
||||||
|
|
||||||
const listener: IGetImageListener = {
|
const listener: IGetImageListener = {
|
||||||
imageReady: (result) => updateImage(result?.data),
|
imageReady: (result) => updateImage(result?.data, requestId),
|
||||||
imageFailed: null
|
imageFailed: null
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -81,7 +88,7 @@ export const LayoutFurniImageView: FC<LayoutFurniImageViewProps> = props =>
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
if(imageResult?.data) updateImage(imageResult.data);
|
if(imageResult?.data) updateImage(imageResult.data, requestId);
|
||||||
}, [ productType, productClassId, direction, extraData, updateImage ]);
|
}, [ productType, productClassId, direction, extraData, updateImage ]);
|
||||||
|
|
||||||
return <Base classNames={ [ 'furni-image' ] } style={ getStyle } { ...rest } />;
|
return <Base classNames={ [ 'furni-image' ] } style={ getStyle } { ...rest } />;
|
||||||
|
|||||||
Reference in New Issue
Block a user