Get user's matches

[AlexD10S]

One possible solution proposed for the issue: https://github.com/SubstrateChess/pallet-chess/issues/23

It allows a frontend app to query only the list of matches for an user with user_matches. For that we are using a new storage map: AccountId and Vector of Matches_Ids:

#[pallet::storage]
    #[pallet::getter(fn user_matches)]
    pub(super) type UserMatches<T: Config> = StorageMap<_, Twox64Concat, T::AccountId, BoundedVec<T::Hash, T::MaxMatchesPerUser>, ValueQuery>;

It also limits the number of matches a user can have, if this is not necessary, this can be skipped using a Vec instead of a BoundedVec.

[bernardoaraujor]

I pushed some commits to fix some formatting issues, but there seems to be too many so I eventually stopped.

I'd suggest running cargo fmt every once in a while, ideally before every commit.

[bernardoaraujor]

Even though I added some comment/suggestions related to the BoundedVec (take vs get), I'm still on the fence about this approach, mainly because it introduces extra computation complexity in places where the benchmarking is already non-trivial (e.g.: binary searches in make_move).

From a runtime development perspective, it would be wiser to use a StorageDoubleMap instead, something like this:

StorageDoubleMap<_, Twox64Concat, T::AccountId, Twox64Concat, T::Hash, (), OptionQuery>;

This would make removing matches O(1), which is much better than doing binary searches over BoundedVec.

In order to find the list of matches for some player, we can simply call iter_key_prefix with a T::AccountId as argument. I have no idea if this is possible on JS however.

[bernardoaraujor]

I would also rename user for player for consistency with the terminology used in the rest of the pallet.

[AlexD10S]

Even though I added some comment/suggestions related to the BoundedVec (take vs get), I'm still on the fence about this approach, mainly because it introduces extra computation complexity in places where the benchmarking is already non-trivial (e.g.: binary searches in make_move).

From a runtime development perspective, it would be wiser to use a StorageDoubleMap instead, something like this:

StorageDoubleMap<_, Twox64Concat, T::AccountId, Twox64Concat, T::Hash, (), OptionQuery>;

This would make removing matches O(1), which is much better than doing binary searches over BoundedVec.

In order to find the list of matches for some player, we can simply call iter_key_prefix with a T::AccountId as argument. I have no idea if this is possible on JS however.

Done! Now removing matches is much better and the code looks cleaner. Still not sure if the solution is the most "elegant" , does makes sense to have a storage with key1, key2 but no value?

[bernardoaraujor]

does makes sense to have a storage with key1, key2 but no value?

This StorageDoubleMap has () as value, and is flagged as OptionQuery.

That means that we query for (player_id, match_id) and get Some(()), then we know that player_id is active on match_id. If we get None, we know player_id is not in it.

As previously mentioned, we're also able to iterate over keys, which is the main purpose of this storage.

So we're still able to retrieve the information we want from UserMatches, but now we're doing optimal search when we need to remove elements from it.

---

I'm writing this comment just to answer your question. I will do a proper review on the code later.