[MLA-1584] Match3 variable board size #5189
Conversation
| /// <summary> | ||
| /// Representation of the AbstractBoard size and number of cell and special types. | ||
| /// </summary> | ||
| public struct BoardSize |
There was a problem hiding this comment.
I'd like review/feedback on this interface before I update the documentation/migration/changelog.
| /// </summary> | ||
| /// <returns></returns> | ||
| public IEnumerable<Move> InvalidMoves() |
There was a problem hiding this comment.
The diff here is messy. InvalidMoves() was removed (previously it was used for action masking, but not anymore).
com.unity.ml-agents.extensions/Runtime/Match3/Match3Actuator.cs
Outdated
Show resolved
Hide resolved
com.unity.ml-agents.extensions/Runtime/Match3/Match3Actuator.cs
Outdated
Show resolved
Hide resolved
com.unity.ml-agents.extensions/Runtime/Match3/Match3Actuator.cs
Outdated
Show resolved
Hide resolved
| writer[offset] = (i == val) ? 1.0f : 0.0f; | ||
| offset++; | ||
| } | ||
| writer.WriteOneHot(offset, r, c, val, m_OneHotSize, isVisual); |
There was a problem hiding this comment.
is this new? I haven't seen it. cool!
| /// </summary> | ||
| /// <param name="boardSize"></param> | ||
| /// <returns></returns> | ||
| public bool IsValidForBoardSize(BoardSize boardSize) |
There was a problem hiding this comment.
I feel like since a move is considered valid or invalid only in the context of a board, this check should go in the board.
There was a problem hiding this comment.
yeah I agree with this, sounds more natural to have Board.IsValidMove(Move move)
There was a problem hiding this comment.
Talked to @surfnerd offline, we agreed on InRangeForBoard. I wanted to separate moved that we know are "invalid" because they go off the current board size, vs. moves that are invalid due to specific game logic.
|
|
||
| m_CurrentBoardSize = new BoardSize |
There was a problem hiding this comment.
sounds a bit weird that BoardSize has attributes like NumCellTypes and NumSpecialTypes. BoardConfig?
There was a problem hiding this comment.
Yeah, that's what I was hoping for feedback on. BoardSpec?
There was a problem hiding this comment.
Also, maybe it's convoluted logic, but the NumCellTypes and NumSpecialTypes define the "depth" of the board observations.
There was a problem hiding this comment.
yea BoardSpec sounds good, so that you can say MaxBoardSpec for max size.
But you convinced me with the "depth" logic. I'm okay with both, leaning towards BoardSpec.
There was a problem hiding this comment.
I think I convinced myself too :) I'm leaning towards BoardSize; I think it makes a more sense to talk about "maximum" for board sizes, but not for specs.
| @@ -3,38 +3,65 @@ | |||
| <img src="images/match3.png" align="center" width="3000"/> | |||
|
|
|||
| ## Overview | |||
| One of the main feedback we get is to illustrate more real game examples using ML-Agents. We are excited to provide an example implementation of Match-3 using ML-Agents and additional utilities to integrate ML-Agents with Match-3 games. | |||
| One of the main feedback we get is to illustrate more real game examples using ML-Agents. We are excited to provide an | |||
There was a problem hiding this comment.
Mostly line breaks here.
Proposed change(s)
TODO: will update the documentation and migration on this PR soon, but I wanted feedback on the AbstractBoard changes first.
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
https://jira.unity3d.com/browse/MLA-1584
https://docs.google.com/drawings/d/1pDlPgWyesTU8rFX1KiarsFaD5Ypwopr0pIr1I-NV_X8/edit (match3 move enumeration cheatsheet)
Types of change(s)
Checklist
Other comments