Repository navigation
Implement Game.set_actions to declare the action list of an information set. - #1040
Conversation
d-kad
added
c++
# Conflicts: # src/games/gametree.cc # tests/test_infosets.py
tturocy
left a comment
tturocy
left a comment
There was a problem hiding this comment.
This is mostly looking OK, but we need to enhance the test coverage - specifically missing would seem to be permutations of more than two actions, or moving actions - and specifically making sure that when actions are moved the whole subtree underneath them goes with it (at the moment looks like only actions above terminal nodes are tried?)
…by remove InsertAction from C++ as now fully superseded.
…changed The cache was only rebuilt when the total node count changed, on the assumption that a same-count edit could only be a relabel. But an edit that drops one node and creates another in the same operation (e.g. simultaneously adding and removing actions at an information set) also leaves the count unchanged while swapping out which nodes exist, so the stale map got indexed with an unrecognized node and null-dereferenced. Now also check that every node in the freshly computed layout is one the cache already has an entry for. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ctions Replace the fixed-row "Move properties" dialog with a variable-length action editor backed by SetActions/RelabelActions: actions can now be added, reordered, and deleted (reversibly, until OK is clicked -- a newly added action has nothing to preserve, so deleting one removes it outright instead) directly in the dialog. New, renamed, and invalid rows are colour- and weight-coded so their status doesn't depend on colour perception alone. This supersedes the standalone "Insert action" menu command, now removed along with its GameDocument command. Renamed the dialog (and its context-menu label) to "Edit move" to match, and documented the new behavior in doc/gui.efg.rst. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@rahulsavani @d-kad I have drafted a new "Edit move" dialog which allows the exercise of the whole of Please have a go and comment as it surely can be improved, though I think it's a big step up from what we had! |
|
I tested the edit move dialog, mostly good, the only bug I found is that reassigning the player raises "Dereferencing an invalidated object" 
|
OnEditMove held onto a single GameInfoset handle across the whole operation, but reassigning the player calls DoSetPlayer, which reforms the information set's members into a brand-new GameInfosetRep via MakeInfoset -- invalidating the original handle, since it's left with no members. The rest of the function then went on to dereference that stale handle (IsChanceInfoset(), and the DoSetActions() call), raising "Dereferencing an invalidated object" instead of completing. Re-fetch the information set through the (still valid) selected node after a player reassignment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # src/games/gametree.cc
Issues closed by this PR
add_action/delete_actioninto single operation #378Description of the changes in this PR
Adds
Game.set_actions(infoset, actions, probs=None, drop=False, add=True), declaring the full orderedaction list of an information set, matching by label. Entries matching current labels keep those actions and
their subtrees, in the declared position, so listing the current labels in a new order reorders the actions;
entries matching no current action create new actions with terminal stubs at every member;
current actions not declared are deleted along with their subtrees.
Deletion is destructive, so
dropmust be set to confirm it.Game.add_actionandGame.delete_actionare removed.At an event, the probability distribution is declared with the actions:
probstakes a sequence or a mapping,exactly as
make_eventdoes, resolved against the declaredactions, and sum to one. A pure reorderingneeds no
probs, since each probability stays with its action. Deleting an action therefore never renormalizes: the distribution over what remains is explicitly declared.The last commit removes
DeleteActionandNormalizeChanceProbs(its only caller) from the C++ core: afterGame.delete_action's removal neither has any caller in the tree.@tturocy let me know if I should drop this last commit if you would rather keep them.