Proposal: Make return values nullable for Gif.toSprite and TileMap.getTileByIndex methods

[ambmcmdmem626]

Version: 0.29.3

Premise

If an index outside the array is specified for the following two methods,
either undefined is returned (TileMap.getTileByIndex) or an exception is thrown (Gif.toSprite).

* Content of the Exception: TypeError: Cannot read properties of undefined (reading 'toSprite')

• Excalibur/src/engine/Resources/Gif.ts

  Lines 79 to 86 in 2e4341f

  	/**
  	* Return a frame of the gif as a sprite by id
  	* @param id
  	*/
  	public toSprite(id: number = 0): Sprite {
  	const sprite = this._textures[id].toSprite();
  	return sprite;
  	}

• Excalibur/src/engine/TileMap/TileMap.ts

  Lines 459 to 464 in 2e4341f

  	/**
  	* Returns the {@apilink Tile} by index (row major order)
  	*/
  	public getTileByIndex(index: number): Tile {
  	return this.tiles[index];
  	}

Main topic

Is there a reason why the return values of these methods are not nullable?

I believe the issue can be addressed by modifying the code as shown below:

• For the comments added to the JSDoc, I referred to TileMap.getTileByPoint()( link ).
• If we decide to implement this change, we should also add corresponding tests, in accordance with the CONTRIBUTING.md.

Suggested Code Changes

// src/engine/Resources/Gif.ts

/**
 * Return a frame of the gif as a sprite by id
 * @param id
 * returns `null` if no Sprite was found.
 */
public toSprite(id: number = 0): Sprite | null {
  const sprite = this._textures[id]?.toSprite() ?? null;
  return sprite;
}

// src/engine/TileMap/TileMap.ts

/**
 * Returns the {@apilink Tile} by index (row major order)
 * returns `null` if no Tile was found.
 */
public getTileByIndex(index: number): Tile | null {
  return this.tiles[index] ?? null;
}

Thank you so much for reading this!

[eonarheim]

@ambmcmdmem626 Thanks for the issue, I totally agree!

[ambmcmdmem626]

@eonarheim
Thank you very much for your reply and for addressing this issue in #3168!
I will proceed to close this discussion!

Additionally, I apologize if this is already known,
but it might be useful to consider adding "noUncheckedIndexedAccess" to tsconfig.json to easily detect such issues in the future. :)

Example

const array = [0, 1, 2];

// Without `noUncheckedIndexedAccess`
const element = array[0]; // number
const outsideElement = array[10]; // number

// With `noUncheckedIndexedAccess`
const element = array[0]; // number | undefined
const outsideElement = array[10]; // number | undefined

[eonarheim]

@ambmcmdmem626 Oh for sure, I was wondering why that wasn't happening like I expected! Thanks for pointing that out!