felangel / equatable

A Dart package that helps to implement value based equality without needing to explicitly override == and hashCode.
https://pub.dev/packages/equatable
MIT License
920 stars 102 forks source link

Equal not working as expected #138

Closed javicerezuela closed 2 years ago

javicerezuela commented 2 years ago

Describe the bug I am using flutter bloc for state management, and I have found that sometimes two different states are treated as equals

To Reproduce This is the state class

enum GameStatus { initial, playing, roundFinished, gameFinished }
class GameState extends Equatable {
  final GameStatus status;
  final int startTime;
  final int timeRemaining;
  final int totalTime;
  final List<Letter> letters;
  final Map<int, Letter> forced = {};
  final List<Letter> currentWord;
  final Map<AbilityType, Ability> abilities;
  final int level;
  final int points;
  final bool usingAbility;
  final AbilityType usedAbility;

  GameState(
      {this.status = GameStatus.initial,
        this.startTime=Config.startingSeconds,
      this.timeRemaining = 45,
      this.totalTime = 0,
      this.letters = const [],
      this.currentWord = const [],
      this.abilities = const {},
      this.level = 1,
      this.points = 0,
      this.usingAbility = false,
      this.usedAbility = AbilityType.rest});

  GameState copyWith({
    GameStatus? status,
    int? startTime,
    int? timeRemaining,
    int? totalTime,
    List<Letter>? letters,
    Map<int, Letter>? forced,
    List<Letter>? currentWord,
    Map<AbilityType, Ability>? abilities,
    int? level,
    int? points,
    bool? usingAbility,
    AbilityType? usedAbility
}){
    return GameState(
        status : status ?? this.status,
        startTime: startTime?? this.startTime,
        timeRemaining: timeRemaining??  this.timeRemaining,
        totalTime : totalTime?? this.totalTime,
        letters: letters ?? this.letters,
        currentWord : currentWord ?? this.currentWord,
        abilities : abilities ?? this.abilities,
        level : level ?? this.level,
        points : points ?? this.points,
        usingAbility : usingAbility ?? this.usingAbility,
        usedAbility : usedAbility ?? this.usedAbility
    );
  }

  @override
  List<Object?> get props => [
        {
          status,
          startTime,
          timeRemaining,
          totalTime,
          letters,
          forced,
          currentWord,
          abilities,
          level,
          points,
          usingAbility,
          usedAbility
        }
      ];
}

Now you can run this code

var state1=GameState(
        status: GameStatus.playing,
        timeRemaining: 1,
        letters: List.empty(),
        abilities: const {},
        level: 1,
        points: 0,
        currentWord: List.empty()
    );
    var state2=state1.copyWith(timeRemaining: 0,);

    assert (state1!=state2); //this assert fails

Expected behavior state1 and state2 should be different

Version Dart SDK version: 2.16.2 (stable) (Tue Mar 22 13:15:13 2022 +0100) on "windows_x64"

miloszratajczyk commented 2 years ago

The states are equal, because in your GameState class you override props to be a list of Sets (the curly brackets). Instead, it should be:

@override
  List<Object?> get props => [
        status,
        startTime,
        timeRemaining,
        totalTime,
        letters,
        forced,
        currentWord,
        abilities,
        level,
        points,
        usingAbility,
        usedAbility,
      ];
javicerezuela commented 2 years ago

@milouw thanks for the answer, I see the problem, I will change the code! but it should be working anyway, right? the two list of sets are different, so the assert should not fail. Or am I missing something?

miloszratajczyk commented 2 years ago

It would work if you set the timeRemaining to 50 for example, but since you set it to 0 it gets ignored because 0 (from points) is already in the set. The same goes for 1 (from level). So in the end, Equatable compares two sets that look something like {0, 1, false} and the change is unnoticeable.

javicerezuela commented 2 years ago

Ok, now I understand, thanks!