Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Replace KnownIsNot* with *IsPlausible in pyhanabi.h. #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ghost
Copy link

@ghost ghost commented Mar 4, 2019

This matches what pyhanabi.py was calling and what pyhanabi.cc was implementing.
It applies to both color and rank.

Also, change them (along with *WasHinted) to return bool instead of int since that's what the underlying functions return.

b-marks added 2 commits March 3, 2019 20:19
This matches what pyhanabi.py was calling and what pyhanabi.cc was implementing.
It applies to both color and rank.
This seems okay since this information is also available in the vectorized observation.
@NeilBurch
Copy link
Contributor

Fix for ColorIsPlausible and RankIsPlausible already added separately: I looked at issues before pull requests. Thank you for also catching this.

Re. bool vs int, I would prefer to leave the use of int-as-bool in the CFFI interface: bool support in CFFI seemed both newer, and a bit rougher compared to support for int. For example, one supposedly up-to-date machine I tested on had cffi 1.9, which does not seem to support C++ bool.

Re. obs_dict, I would prefer to leave the RL interface untouched unless necessary, even if it might not be the most aesthetically pleasing choice. The extended hint information is exposed in both the vectorized observation and the pyhanabi observation. If I was to change the RL interface, my personal preference would actually be to remove almost all of the obs_dict entries as they're duplicates of the HanabiObservation object.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants