From aeb6a174ef40d64b057f9b6a37e62242fa6925ce Mon Sep 17 00:00:00 2001 From: Nicolas Echezarreta Date: Mon, 14 Oct 2024 07:34:45 -0700 Subject: [PATCH] fix (and reworkit a little bit) the Layout Grid (#1018) --- .../SceneInspector/Layout/Layout.tsx | 104 ++++------- .../SceneInspector/Layout/utils.spec.ts | 98 +++------- .../SceneInspector/Layout/utils.ts | 175 ++++++++++-------- 3 files changed, 164 insertions(+), 213 deletions(-) diff --git a/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/Layout.tsx b/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/Layout.tsx index 650ba16e4..f87ef4f6a 100644 --- a/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/Layout.tsx +++ b/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/Layout.tsx @@ -1,5 +1,5 @@ import { areConnected } from '@dcl/ecs' -import { useCallback, useState } from 'react' +import { useCallback, useMemo, useState } from 'react' import { AiOutlineInfoCircle as InfoIcon } from 'react-icons/ai' import { Dropdown } from '../../../ui' @@ -9,16 +9,17 @@ import { Grid, Props as GridProps } from './Grid' import { coordToStr, - getCoordinates, getMinMaxFromOrderedCoords, getOption, - getLayoutInfo, stringifyGridError, getLayoutInfoFromString, strToCoord, transformCoordsToString, getEnabledCoords, - hasCoord + hasCoord, + getGridInfo, + isCoord, + generateGridFrom } from './utils' import { getAxisLengths } from './Grid/utils' import { ModeAdvanced } from './ModeAdvanced' @@ -30,59 +31,35 @@ import './Layout.css' type Coords = GridProps['coords'][0] function Layout({ value, onChange }: Props) { - const currentLayout = getLayoutInfo(value.parcels) - const coordinates = getCoordinates(currentLayout.min, currentLayout.max) - - const [grid, setGrid] = useState(coordinates) - const [disabled, setDisabled] = useState(new Set()) + const { grid: _grid } = useMemo(() => getGridInfo(value.parcels), [value.parcels]) + const [grid, setGrid] = useState(_grid) const [mode, setMode] = useState(Mode.GRID) const [base, setBase] = useState(value.base) - const [gridMin, gridMax] = getMinMaxFromOrderedCoords(grid) - const axisLengths = getAxisLengths(grid) - const enabledCoords = getEnabledCoords(grid, disabled) + const [gridMin, gridMax] = useMemo(() => getMinMaxFromOrderedCoords(grid), [grid]) + const axisLengths = useMemo(() => getAxisLengths(grid), [grid]) + const enabledCoords = useMemo(() => getEnabledCoords(grid), [grid]) const numberOfCoords = enabledCoords.length - const handleGridChange = (type: keyof Coords) => (e: React.ChangeEvent) => { - // this should also work for negative parcels... - const num = Number(e.target.value) - const grixMaxAxis = gridMax[type] - const axisLength = axisLengths[type] - const diff = Math.abs(axisLength - num) - const value = num > axisLength ? grixMaxAxis + diff : grixMaxAxis - diff - const newMax: Coords = { ...gridMax, [type]: value } - return setGrid(getCoordinates(gridMin, newMax)) - } - - const isTileDisabled = useCallback( - (coord: Coords) => { - const str = coordToStr(coord) - return disabled.has(str) + const handleGridChange = useCallback( + (type: keyof Coords) => (e: React.ChangeEvent) => { + const num = Number(e.target.value) + const axisLength = axisLengths[type] + const diff = Math.abs(axisLength - num) + const value = num > axisLength ? gridMax[type] + diff : gridMax[type] - diff + const newMax: Coords = { ...gridMax, [type]: value } + setGrid(generateGridFrom(grid, gridMin, newMax)) }, - [grid, disabled] + [grid, gridMin, gridMax, axisLengths] ) - // const isTileDisconnected = useCallback((coord: Coords) => {}, [grid, disabled]) + const isTileDisabled = useCallback((coord: Coords) => !hasCoord(enabledCoords, coord), [enabledCoords]) - const isBaseTile = useCallback( - (coord: Coords) => { - return coord.x === base.x && coord.y === base.y - }, - [base] - ) + const isBaseTile = useCallback((coord: Coords) => coord.x === base.x && coord.y === base.y, [base]) - const handleTileClick = useCallback( - (coord: Coords) => { - const str = coordToStr(coord) - if (disabled.has(str)) { - disabled.delete(str) - } else { - disabled.add(str) - } - setDisabled(new Set(disabled)) - }, - [grid, disabled] - ) + const handleTileClick = useCallback((coord: Coords) => { + setGrid((prevGrid) => prevGrid.map(($) => (isCoord($, coord) ? { ...$, disabled: !$.disabled } : $))) + }, []) const handleAdvancedConfirm = useCallback( (value: ModeAdvancedValue) => { @@ -91,46 +68,35 @@ function Layout({ value, onChange }: Props) { x: length.x > MAX_AXIS_PARCELS ? min.x + Math.min(MAX_AXIS_PARCELS, max.x) : max.x, y: length.y > MAX_AXIS_PARCELS ? min.y + Math.min(MAX_AXIS_PARCELS, max.y) : max.y } - const parcels = getCoordinates(min, clampMax) - const base = strToCoord(value.base) || min - setGrid(parcels) - setBase(base) + setGrid(generateGridFrom(grid, min, clampMax)) + setBase(strToCoord(value.base) || min) }, - [grid, base, disabled] + [grid] ) const applyCurrentState = useCallback(() => { onChange({ parcels: enabledCoords, base }) - }, [grid, base, disabled]) + }, [enabledCoords, base]) - const handleModeChange = useCallback( - (mode: Mode) => () => { - setMode(mode) - }, - [mode] - ) + const handleModeChange = useCallback((mode: Mode) => () => setMode(mode), []) - const getGridError = useCallback((): GridError | null => { + const gridError = useMemo((): GridError | null => { if (numberOfCoords <= 0) return GridError.NUMBER_OF_PARCELS if (!areConnected(enabledCoords)) return GridError.NOT_CONNECTED if (!hasCoord(enabledCoords, base)) return GridError.MISSING_BASE_PARCEL return null - }, [grid, base, disabled]) + }, [numberOfCoords, enabledCoords, base]) - const getTitle = useCallback(() => { + const title = useMemo(() => { if (mode === Mode.ADVANCED) return 'Set Coordinates' return `${numberOfCoords} Parcel${numberOfCoords === 1 ? '' : 's'}` - }, [grid, mode, disabled]) + }, [numberOfCoords, mode]) - const getInstruction = useCallback(() => { + const instruction = useMemo(() => { if (mode === Mode.ADVANCED) return 'Type in the layout coordinates you want to deploy' return 'Click individual tiles to exclude/include them from the layout' }, [mode]) - const title = getTitle() - const instruction = getInstruction() - const gridError = getGridError() - return (
@@ -151,7 +117,7 @@ function Layout({ value, onChange }: Props) { {mode === Mode.ADVANCED ? ( { - it('returns correct parcel info', () => { +describe('getGridInfo', () => { + it('returns correct grid info', () => { const parcels = [ { x: 0, y: 0 }, { x: 0, y: 1 }, @@ -22,9 +21,14 @@ describe('getLayoutInfo', () => { min: { x: 0, y: 0 }, max: { x: 1, y: 1 }, length: { x: 1, y: 1 }, - parcels: parcels + grid: [ + { x: 0, y: 1, disabled: false }, + { x: 1, y: 1, disabled: false }, + { x: 0, y: 0, disabled: false }, + { x: 1, y: 0, disabled: false } + ] } - expect(getLayoutInfo(parcels)).toEqual(expected) + expect(getGridInfo(parcels)).toEqual(expected) }) }) @@ -35,51 +39,33 @@ describe('getLayoutInfoFromString', () => { min: { x: 0, y: 0 }, max: { x: 1, y: 1 }, length: { x: 1, y: 1 }, - parcels: [ - { x: 0, y: 0 }, - { x: 0, y: 1 }, - { x: 1, y: 0 }, - { x: 1, y: 1 } + grid: [ + { x: 0, y: 1, disabled: false }, + { x: 1, y: 1, disabled: false }, + { x: 0, y: 0, disabled: false }, + { x: 1, y: 0, disabled: false } ] } expect(getLayoutInfoFromString(parcelsString)).toEqual(expected) }) }) -describe('getCoordinatesBetweenPoints', () => { - it('returns correct coordinates between two points', () => { +describe('generateCoordinatesBetweenPoints', () => { + it('returns correct coordinates between two points sorted in grid order', () => { const pointA = { x: 0, y: 0 } const pointB = { x: 2, y: 2 } const expected = [ - { x: 0, y: 0 }, - { x: 0, y: 1 }, { x: 0, y: 2 }, - { x: 1, y: 0 }, - { x: 1, y: 1 }, { x: 1, y: 2 }, - { x: 2, y: 0 }, - { x: 2, y: 1 }, - { x: 2, y: 2 } - ] - expect(getCoordinatesBetweenPoints(pointA, pointB)).toEqual(expected) - }) -}) - -describe('getCoordinatesInGridOrder', () => { - it('returns coordinates sorted in grid order', () => { - const unsorted = [ - { x: 0, y: 0 }, - { x: 0, y: 1 }, - { x: 1, y: 0 }, - { x: 1, y: 1 } - ] - const expected = [ + { x: 2, y: 2 }, { x: 0, y: 1 }, { x: 1, y: 1 }, + { x: 2, y: 1 }, { x: 0, y: 0 }, - { x: 1, y: 0 } + { x: 1, y: 0 }, + { x: 2, y: 0 } ] - expect(getCoordinatesInGridOrder(unsorted)).toEqual(expected) + expect(generateCoordinatesBetweenPoints(pointA, pointB)).toEqual(expected) }) }) @@ -185,38 +171,14 @@ describe('transformCoordsToString', () => { { x: 1, y: 1 }, { x: 2, y: 2 } ] - const disabledCoords = new Set() const expected = '0,0 1,1 2,2' - expect(transformCoordsToString(coords, disabledCoords)).toEqual(expected) - }) - - it('filters disabled coordinates', () => { - const coords = [ - { x: 0, y: 0 }, - { x: 1, y: 1 }, - { x: 2, y: 2 } - ] - const disabledCoords = new Set(['1,1']) - const expected = '0,0 2,2' - expect(transformCoordsToString(coords, disabledCoords)).toEqual(expected) + expect(transformCoordsToString(coords)).toEqual(expected) }) it('returns empty string for empty coordinates array', () => { const coords = [] - const disabledCoords = new Set() - const expected = '' - expect(transformCoordsToString(coords, disabledCoords)).toEqual(expected) - }) - - it('returns empty string for all coordinates disabled', () => { - const coords = [ - { x: 0, y: 0 }, - { x: 1, y: 1 }, - { x: 2, y: 2 } - ] - const disabledCoords = new Set(['0,0', '1,1', '2,2']) const expected = '' - expect(transformCoordsToString(coords, disabledCoords)).toEqual(expected) + expect(transformCoordsToString(coords)).toEqual(expected) }) it('handles coordinates with negative values', () => { @@ -225,9 +187,8 @@ describe('transformCoordsToString', () => { { x: 0, y: 0 }, { x: 1, y: 1 } ] - const disabledCoords = new Set(['-1,-1']) - const expected = '0,0 1,1' - expect(transformCoordsToString(coords, disabledCoords)).toEqual(expected) + const expected = '-1,-1 0,0 1,1' + expect(transformCoordsToString(coords)).toEqual(expected) }) it('handles large coordinate values', () => { @@ -235,9 +196,8 @@ describe('transformCoordsToString', () => { { x: 1000000, y: 1000000 }, { x: 2000000, y: 2000000 } ] - const disabledCoords = new Set() const expected = '1000000,1000000 2000000,2000000' - expect(transformCoordsToString(coords, disabledCoords)).toEqual(expected) + expect(transformCoordsToString(coords)).toEqual(expected) }) }) diff --git a/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/utils.ts b/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/utils.ts index 5ecd958e2..3e41ca0b8 100644 --- a/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/utils.ts +++ b/packages/@dcl/inspector/src/components/EntityInspector/SceneInspector/Layout/utils.ts @@ -2,106 +2,115 @@ import { Coords } from '@dcl/ecs' import { GridError, TILE_OPTIONS } from './types' import { parseParcels } from '../utils' -type ParcelInfo = { +export type GridCoord = Coords & { disabled?: boolean } + +export type GridInfo = { min: Coords max: Coords - length: { - x: number - y: number - } - parcels: Coords[] + length: Coords + grid: GridCoord[] } -const DEFAULT_INFO: ParcelInfo = { +const DEFAULT_INFO = { min: { x: 0, y: 0 }, max: { x: 0, y: 0 }, length: { x: 0, y: 0 }, - parcels: [] + grid: [] } -export function getLayoutInfo(parcels: Coords[]): ParcelInfo { +export function getGridInfo(parcels: Coords[]): GridInfo { if (!parcels.length) return DEFAULT_INFO - const info: { min: Coords; max: Coords } = { - min: { x: Infinity, y: Infinity }, - max: { x: -Infinity, y: -Infinity } - } - parcels.forEach((parcel) => { - const { x, y } = parcel - if (info.min.y >= y) { - info.min = { x: Math.min(info.min.x, x), y } - } + const minParcel = { x: Infinity, y: Infinity } + const maxParcel = { x: -Infinity, y: -Infinity } - if (y >= info.max.y) { - info.max = { x: Math.max(info.max.x, x), y } - } + const coordSet = new Set() + + parcels.forEach((parcel) => { + // get min parcel + minParcel.x = Math.min(parcel.x, minParcel.x) + minParcel.y = Math.min(parcel.y, minParcel.y) - return { x, y } + // get max parcel + maxParcel.x = Math.max(parcel.x, maxParcel.x) + maxParcel.y = Math.max(parcel.y, maxParcel.y) + + // push stringified coord to coordSet + coordSet.add(coordToStr(parcel)) }) + const coordinatesInOrder = generateCoordinatesBetweenPoints(minParcel, maxParcel).map(($) => ({ + ...$, + disabled: !coordSet.has(coordToStr($)) + })) + return { - min: info.min, - max: info.max, + min: minParcel, + max: maxParcel, length: { - x: Math.abs(info.max.x) - Math.abs(info.min.x), - y: Math.abs(info.max.y) - Math.abs(info.min.y) + x: Math.abs(maxParcel.x) - Math.abs(minParcel.x), + y: Math.abs(maxParcel.y) - Math.abs(minParcel.y) }, - parcels + grid: coordinatesInOrder } } -/* Parcels string format rules: - ** #1: each coordinate is space-separated - ** #2: each point is comma-separated - ** EX: "0,0 0,1 1,0 1,1" +/** + * Generates an array of coordinates between two points (inclusive). + * The coordinates are generated in grid order, from top-left to bottom-right, + * starting from the maximum y-value to the minimum y-value, and within each row, + * from the minimum x-value to the maximum x-value. + * + * @param {Coords} pointA - The first point (can be any of the two corners of the grid). + * @param {Coords} pointB - The second point (can be any of the two corners of the grid). + * @returns {Coords[]} - An array of coordinates between the two points, inclusive, + * ordered from top-left to bottom-right. */ -export function getLayoutInfoFromString(parcels: string): ParcelInfo { - return getLayoutInfo(parseParcels(parcels)) -} - -export function getCoordinatesBetweenPoints(pointA: Coords, pointB: Coords): Coords[] { +export function generateCoordinatesBetweenPoints(pointA: Coords, pointB: Coords): Coords[] { const coordinates: Coords[] = [] - // ensure pointA is the bottom-left coord - if (pointA.x > pointB.x) { - ;[pointA.x, pointB.x] = [pointB.x, pointA.x] - } - if (pointA.y > pointB.y) { - ;[pointA.y, pointB.y] = [pointB.y, pointA.y] - } + const [minX, maxX] = [Math.min(pointA.x, pointB.x), Math.max(pointA.x, pointB.x)] + const [minY, maxY] = [Math.min(pointA.y, pointB.y), Math.max(pointA.y, pointB.y)] - for (let x = pointA.x; x <= pointB.x; x++) { - for (let y = pointA.y; y <= pointB.y; y++) { - coordinates.push({ x, y }) + for (let y = maxY; y >= minY; y--) { + for (let x = minX; x <= maxX; x++) { + const coord = { x, y } + coordinates.push(coord) } } return coordinates } -/* - ** Sorts the coordinates for grid rendering - ** This means: - ** - X-axis => Lowest to highest - ** - Y-axis => Highest to lowest - ** EX: [{ x: 0, y: 0 }, { x: 0, y: 1 }, { x: 1, y: 0 }, { x: 1, y: 1 }] - ** => [{ x: 0, y: 1 }, { x: 1, y: 1 }, { x: 0, y: 0 }, { x: 1, y: 0 }] +/** + * Generates a grid of coordinates between the specified min and max points. + * Marks coordinates as "disabled" based on the provided grid's existing disabled coordinates. + * + * @param {GridCoord[]} grid - The existing grid of coordinates, some of which may be disabled. + * @param {Coords} min - The minimum coordinate point (bottom-left) to define the grid bounds. + * @param {Coords} max - The maximum coordinate point (top-right) to define the grid bounds. + * @returns {GridCoord[]} - A new grid of coordinates between the min and max points, + * with coordinates marked as disabled if they exist as disabled in the original grid. */ -export function getCoordinatesInGridOrder(coords: Coords[]): Coords[] { - // avoid mutating original coords... - return [...coords].sort((a, b) => { - // first, sort by y-coordinate in descending order - if (a.y !== b.y) return b.y - a.y - // If y-coordinates are the same, sort by x-coordinate in ascending order - return a.x - b.x +export function generateGridFrom(grid: GridCoord[], min: Coords, max: Coords): GridCoord[] { + const disabledCoords = new Set(grid.filter((coord) => coord.disabled).map((coord) => coordToStr(coord))) + + return generateCoordinatesBetweenPoints(min, max).map((coord) => { + const coordStr = coordToStr(coord) + return { + ...coord, + disabled: disabledCoords.has(coordStr) + } }) } -/* - ** Returns coordinates between "min" and "max" in grid order +/* Parcels string format rules: + ** #1: each coordinate is space-separated + ** #2: each point is comma-separated + ** EX: "0,0 0,1 1,0 1,1" */ -export function getCoordinates(min: Coords, max: Coords): Coords[] { - return getCoordinatesInGridOrder(getCoordinatesBetweenPoints(min, max)) +export function getLayoutInfoFromString(parcels: string): GridInfo { + return getGridInfo(parseParcels(parcels)) } /* @@ -120,14 +129,25 @@ export function clampParcels(value: number): number { ** Gets min & max coordinates from grid-ordered coordinates */ export function getMinMaxFromOrderedCoords(coords: Coords[]): [Coords, Coords] { + if (!coords.length) { + return [ + { x: 0, y: 0 }, + { x: 0, y: 0 } + ] + } + + const first = coords[0] + const last = coords[coords.length - 1] return [ - { x: coords[0].x, y: coords[coords.length - 1].y }, - { x: coords[coords.length - 1].x, y: coords[0].y } + { x: first.x, y: last.y }, + { x: last.x, y: first.y } ] } -/* - ** Transform a coordinate to it's string representation +/** + * Converts a coordinate object to its string representation. + * @param {Coords} coords - The coordinate object. + * @returns {string} The string representation of the coordinates. */ export function coordToStr({ x, y }: Coords): string { return `${x},${y}` @@ -144,15 +164,22 @@ export function strToCoord(coord: string): Coords { /* ** Filter out the disabled coordinates */ -export function getEnabledCoords(coords: Coords[], disabledCoords: Set) { - return coords.filter(($) => !disabledCoords.has(coordToStr($))) +export function getEnabledCoords(coords: GridCoord[]) { + return coords.filter(($) => $.disabled === false) +} + +/* + ** Returns true if the two coords are equal + */ +export function isCoord(coord1: Coords, coord2: Coords): boolean { + return coord1.x === coord2.x && coord1.y === coord2.y } /* ** Find a specific coordinate in the list of coordinates */ export function findCoord(coords: Coords[], needle: Coords) { - return coords.find(($) => $.x === needle.x && $.y === needle.y) + return coords.find(($) => isCoord($, needle)) } /* @@ -163,13 +190,11 @@ export function hasCoord(coords: Coords[], needle: Coords) { } /* - ** Transform list of coordinates to their string-representation form and filters the - ** disabled ones + ** Transform list of coordinates to their string-representation form */ -export function transformCoordsToString(coords: Coords[], disabledCoords: Set) { +export function transformCoordsToString(coords: Coords[]) { return coords .map(($) => coordToStr($)) // map to string - .filter(($) => !disabledCoords.has($)) // remove disabled coords .join(' ') }