Skip to content

Commit

Permalink
Add more bounds checking to collections. Prevents a number of crashes
Browse files Browse the repository at this point in the history
  • Loading branch information
@ethanmoffat
ethanmoffat committed Mar 7, 2022
1 parent 114e6e4 commit a767c16
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 48 deletions.
10 changes: 9 additions & 1 deletion EOLib.IO/Map/Matrix.cs
View file
Expand Up @@ -11,6 +11,8 @@ public class Matrix<T> : IReadOnlyMatrix<T>

private readonly T[,] _arr;

private readonly T _default;

public int Rows { get; }
public int Cols { get; }

Expand All @@ -24,6 +26,7 @@ private Matrix(int rows, int cols)
public Matrix(int rows, int cols, T defaultValue)
: this(rows, cols)
{
_default = defaultValue;
Fill(defaultValue);
}

Expand Down Expand Up @@ -56,7 +59,12 @@ public T[] GetRow(int rowIndex)

public T this[int row, int col]
{
get { return _arr[row, col]; }
get
{
if (row < 0 || row >= Rows || col < 0 || col >= Cols)
return _default;
return _arr[row, col];
}
set { _arr[row, col] = value; }
}

Expand Down
34 changes: 19 additions & 15 deletions EndlessClient/Rendering/Character/CharacterAnimationActions.cs
View file
Expand Up @@ -57,9 +57,7 @@ public void StartWalking()
return;

Animator.StartMainCharacterWalkAnimation();
ShowWaterSplashiesIfNeeded(CharacterActionState.Walking,
_characterRepository.MainCharacter,
_characterRendererProvider.MainCharacterRenderer);
ShowWaterSplashiesIfNeeded(CharacterActionState.Walking, _characterRepository.MainCharacter.ID);
}

public void StartAttacking()
Expand All @@ -68,9 +66,7 @@ public void StartAttacking()
return;

Animator.StartMainCharacterAttackAnimation();
ShowWaterSplashiesIfNeeded(CharacterActionState.Attacking,
_characterRepository.MainCharacter,
_characterRendererProvider.MainCharacterRenderer);
ShowWaterSplashiesIfNeeded(CharacterActionState.Attacking, _characterRepository.MainCharacter.ID);
}

public void StartOtherCharacterWalkAnimation(int characterID, byte destinationX, byte destinationY, EODirection direction)
Expand All @@ -79,11 +75,8 @@ public void StartOtherCharacterWalkAnimation(int characterID, byte destinationX,
return;

Animator.StartOtherCharacterWalkAnimation(characterID, destinationX, destinationY, direction);

ShowWaterSplashiesIfNeeded(CharacterActionState.Walking,
_currentMapStateProvider.Characters.Single(x => x.ID == characterID),
_characterRendererProvider.CharacterRenderers[characterID]);


ShowWaterSplashiesIfNeeded(CharacterActionState.Walking, characterID);
_spikeTrapActions.HideSpikeTrap(characterID);
_spikeTrapActions.ShowSpikeTrap(characterID);
}
Expand All @@ -94,9 +87,7 @@ public void StartOtherCharacterAttackAnimation(int characterID)
return;

Animator.StartOtherCharacterAttackAnimation(characterID);
ShowWaterSplashiesIfNeeded(CharacterActionState.Attacking,
_currentMapStateProvider.Characters.Single(x => x.ID == characterID),
_characterRendererProvider.CharacterRenderers[characterID]);
ShowWaterSplashiesIfNeeded(CharacterActionState.Attacking, characterID);
}

public void NotifyWarpLeaveEffect(short characterId, WarpAnimation anim)
Expand Down Expand Up @@ -178,8 +169,21 @@ public void NotifyEarthquake(byte strength)
mapRenderer.StartEarthquake(strength);
}

private void ShowWaterSplashiesIfNeeded(CharacterActionState action, ICharacter character, ICharacterRenderer characterRenderer)
private void ShowWaterSplashiesIfNeeded(CharacterActionState action, int characterID)
{
var character = characterID == _characterRepository.MainCharacter.ID
? _characterRepository.MainCharacter
: _currentMapStateProvider.Characters.SingleOrDefault(x => x.ID == characterID);

var characterRenderer = characterID == _characterRepository.MainCharacter.ID
? _characterRendererProvider.MainCharacterRenderer
: _characterRendererProvider.CharacterRenderers.ContainsKey(characterID)
? _characterRendererProvider.CharacterRenderers[characterID]
: null;

if (character == null || characterRenderer == null)
return;

var rp = character.RenderProperties;
if (action == CharacterActionState.Attacking)
{
Expand Down
12 changes: 2 additions & 10 deletions EndlessClient/Rendering/Character/CharacterAnimator.cs
View file
Expand Up @@ -230,8 +230,8 @@ private void AnimateCharacterWalking()

private ICharacterRenderProperties AnimateOneWalkFrame(ICharacterRenderProperties renderProperties)
{
var isSteppingStone = (IsInBounds(renderProperties, false) && _currentMapProvider.CurrentMap.Tiles[renderProperties.MapY, renderProperties.MapX] == TileSpec.Jump)
|| (IsInBounds(renderProperties, true) && _currentMapProvider.CurrentMap.Tiles[renderProperties.GetDestinationY(), renderProperties.GetDestinationX()] == TileSpec.Jump);
var isSteppingStone = _currentMapProvider.CurrentMap.Tiles[renderProperties.MapY, renderProperties.MapX] == TileSpec.Jump
|| _currentMapProvider.CurrentMap.Tiles[renderProperties.GetDestinationY(), renderProperties.GetDestinationX()] == TileSpec.Jump;

var nextFrameRenderProperties = renderProperties.WithNextWalkFrame(isSteppingStone);
if (nextFrameRenderProperties.CurrentAction != CharacterActionState.Walking)
Expand All @@ -244,14 +244,6 @@ private ICharacterRenderProperties AnimateOneWalkFrame(ICharacterRenderPropertie
return nextFrameRenderProperties;
}

private bool IsInBounds(ICharacterRenderProperties renderProperties, bool dest)
{
var mapProps = _currentMapProvider.CurrentMap.Properties;
var x = dest ? renderProperties.GetDestinationX() : renderProperties.MapX;
var y = dest ? renderProperties.GetDestinationY() : renderProperties.MapY;
return x < mapProps.Width && x >= 0 && y < mapProps.Height && y >= 0;
}

#endregion

#region Attack Animation
Expand Down
12 changes: 2 additions & 10 deletions EndlessClient/Rendering/Character/CharacterRenderer.cs
View file
Expand Up @@ -362,8 +362,8 @@ private bool GetIsSteppingStone(ICharacterRenderProperties renderProps)
if (_gameStateProvider.CurrentState != GameStates.PlayingTheGame)
return false;

return (IsInBounds(renderProps, false) && _currentMapProvider.CurrentMap.Tiles[renderProps.MapY, renderProps.MapX] == TileSpec.Jump)
|| (IsInBounds(renderProps, true) && renderProps.IsActing(CharacterActionState.Walking) && _currentMapProvider.CurrentMap.Tiles[renderProps.GetDestinationY(), renderProps.GetDestinationX()] == TileSpec.Jump);
return _currentMapProvider.CurrentMap.Tiles[renderProps.MapY, renderProps.MapX] == TileSpec.Jump
|| (renderProps.IsActing(CharacterActionState.Walking) && _currentMapProvider.CurrentMap.Tiles[renderProps.GetDestinationY(), renderProps.GetDestinationX()] == TileSpec.Jump);
}

private int GetSteppingStoneOffset(ICharacterRenderProperties renderProps)
Expand All @@ -384,14 +384,6 @@ private int GetSteppingStoneOffset(ICharacterRenderProperties renderProps)
return 0;
}

private bool IsInBounds(ICharacterRenderProperties renderProperties, bool dest)
{
var mapProps = _currentMapProvider.CurrentMap.Properties;
var x = dest ? renderProperties.GetDestinationX() : renderProperties.MapX;
var y = dest ? renderProperties.GetDestinationY() : renderProperties.MapY;
return x < mapProps.Width && x >= 0 && y < mapProps.Height && y >= 0;
}

#endregion

#region Effects
Expand Down
32 changes: 21 additions & 11 deletions EndlessClient/Rendering/Character/CharacterRendererUpdater.cs
View file
Expand Up @@ -85,11 +85,13 @@ private void CreateOtherCharacterRenderersAndCacheProperties()
}
else if (cached.Value != character)
{
_characterRendererRepository.CharacterRenderers[id].Character = character;
if (_characterRendererRepository.CharacterRenderers.ContainsKey(id))
_characterRendererRepository.CharacterRenderers[id].Character = character;
_characterStateCache.UpdateCharacterState(id, character);
}

if (_characterRendererRepository.NeedsWarpArriveAnimation.Contains(id))
if (_characterRendererRepository.NeedsWarpArriveAnimation.Contains(id) &&
_characterRendererRepository.CharacterRenderers.ContainsKey(id))
{
_characterRendererRepository.CharacterRenderers[id].ShowWarpArrive();
_characterRendererRepository.NeedsWarpArriveAnimation.Remove(id);
Expand All @@ -116,15 +118,19 @@ private void RemoveStaleCharacters()

foreach (var id in staleIDs)
{
if (_characterRendererRepository.CharacterRenderers[id].EffectIsPlaying())
_characterStateCache.RemoveCharacterState(id);

if (_characterRendererRepository.CharacterRenderers.ContainsKey(id))
{
_characterRendererRepository.CharacterRenderers[id].Visible = false;
continue;
}
if (_characterRendererRepository.CharacterRenderers[id].EffectIsPlaying())
{
_characterRendererRepository.CharacterRenderers[id].Visible = false;
continue;
}

_characterStateCache.RemoveCharacterState(id);
_characterRendererRepository.CharacterRenderers[id].Dispose();
_characterRendererRepository.CharacterRenderers.Remove(id);
_characterRendererRepository.CharacterRenderers[id].Dispose();
_characterRendererRepository.CharacterRenderers.Remove(id);
}
}
}

Expand All @@ -144,8 +150,12 @@ private void UpdateDeadCharacters()
_characterStateCache.RemoveDeathStartTime(character.ID);
_characterStateCache.RemoveCharacterState(character.ID);

_characterRendererRepository.CharacterRenderers[character.ID].Dispose();
_characterRendererRepository.CharacterRenderers.Remove(character.ID);
if (_characterRendererRepository.CharacterRenderers.ContainsKey(character.ID))
{
_characterRendererRepository.CharacterRenderers[character.ID].Dispose();
_characterRendererRepository.CharacterRenderers.Remove(character.ID);
}

deadCharacters.Add(character);
}
}
Expand Down
2 changes: 1 addition & 1 deletion EndlessClient/Rendering/NPC/NPCActions.cs
View file
Expand Up @@ -117,7 +117,7 @@ private void ShoutSpellCast(int playerId)
{
if (_characterRendererRepository.MainCharacterRenderer.Character.ID == playerId)
_characterRendererRepository.MainCharacterRenderer.ShoutSpellCast();
else
else if (_characterRendererRepository.CharacterRenderers.ContainsKey(playerId))
_characterRendererRepository.CharacterRenderers[playerId].ShoutSpellCast();
}

Expand Down

0 comments on commit a767c16

Please sign in to comment.