Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capability exercise cost #1777

Merged
merged 10 commits into from
Apr 25, 2024
Merged

Capability exercise cost #1777

merged 10 commits into from
Apr 25, 2024

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Feb 24, 2024

Closes #1684
Closes #1262

Demo

A simple "puzzle" that makes use of consumables:

scripts/play.sh -i data/scenarios/Testing/1777-capability-cost.yaml --autoplay

Demo of enabled commands and costs display in left pane:

scripts/play.sh -i data/scenarios/Testing/1262-display-device-commands.yaml

Screenshot from 2024-03-01 22-39-12

In this PR

  • Supports specifying capabilities both as plain lists and as maps from capabilities to ingredients in YAML
  • JSON Schema support for both capability specification styles
  • New integration test
  • Removed redundant tshow implementation from Swarm.Doc.Util

Entity lookup approaches

The cost of exercising a capability in terms of "ingredients" is specified by the _entityCapabilities field within an Entity definition. Each ingredient itself is an entity, specified by name.
For many purposes, having ingredients just of type EntityName is sufficient. But for Swarm.Game.Recipe.findLacking in particular, the ingredients list must be actual Entity objects, not just names of entities. So at some point the names need to be looked up in the global entity map to be promoted to Entity objects.

The full list of entities is not available at Entity parse time to look up an ingredient entity by name, so we cannot store ingredients lists of type (Count, Entity) within a parent Entity object.

Approaches considered were:

  • Store a copy of the entityMap in RobotR, for use by the equippedDevices lens
  • Introduce a type parameter to Entity representing a "parse phase"
  • Allow a redundant "entity lookup" (and validation) at command execution time

Store entityMap in RobotR

One approach explored was to add a field to RobotR:

  , _globalEntityMap :: EntityMap

This allowed the equippedDevices lens implementation to promote the EntityNames to Entitys when setting the value of the _robotCapabilities field. However, it was rather invasive as it entailed threading the EntityMap through many new code paths.

Entity type parameter

Currently, Entity has a field:

_entityCapabilities :: SingleEntityCapabilities EntityName

This would entail a huge refactoring, with:

data Entity e = Entity
  ...
  , _entityCapabilities :: SingleEntityCapabilities e

At initial parse time we would obtain a list of Entity EntityName, but somewhere later during Scenario parse time, we can do another pass to obtain Entity Entity objects.

This would at least have the advantage of doing the entity lookup/validation on ingredient lists in exactly one place, at parse time.

Defer EntityName -> Entity promotion to command execution time

This is what is implemented in this PR. The global set of capability costs is validated at scenario parse time. But it is also redundantly validated in the payExerciseCost function, which is not ideal.

@kostmo kostmo force-pushed the feature/capability-cost branch 2 times, most recently from e817921 to 28bc252 Compare February 24, 2024 07:10
@kostmo kostmo changed the title [WIP] Capability exercise cost Capability exercise cost Feb 29, 2024
@kostmo kostmo force-pushed the feature/capability-cost branch 2 times, most recently from 7624011 to 01ac86a Compare February 29, 2024 03:25
@kostmo kostmo force-pushed the feature/capability-cost branch 2 times, most recently from 62ee143 to 1ca0385 Compare February 29, 2024 06:31
src/swarm-scenario/Swarm/Game/Device.hs Outdated Show resolved Hide resolved
src/swarm-scenario/Swarm/Game/Device.hs Outdated Show resolved Hide resolved
src/swarm-scenario/Swarm/Game/Device.hs Outdated Show resolved Hide resolved
@kostmo kostmo requested a review from byorgey February 29, 2024 17:59
@kostmo kostmo marked this pull request as ready for review February 29, 2024 17:59
@kostmo kostmo force-pushed the feature/capability-cost branch 4 times, most recently from 9660bb0 to 4158726 Compare March 2, 2024 06:39
@byorgey byorgey mentioned this pull request Mar 8, 2024
@kostmo kostmo force-pushed the feature/capability-cost branch 2 times, most recently from 1e44c75 to 56b2a35 Compare March 11, 2024 20:53
src/swarm-scenario/Swarm/Game/Device.hs Outdated Show resolved Hide resolved
src/swarm-scenario/Swarm/Game/Device.hs Show resolved Hide resolved
src/swarm-scenario/Swarm/Game/Device.hs Outdated Show resolved Hide resolved
src/swarm-scenario/Swarm/Game/Device.hs Show resolved Hide resolved
getCapabilitySet :: Capabilities e -> Set Capability
getCapabilitySet (Capabilities m) = M.keysSet m

type SingleEntityCapabilities e = Capabilities (ExerciseCost e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type SingleEntityCapabilities e = Capabilities (ExerciseCost e)
-- | Records an 'ExerciseCost', i.e. list of consumed ingredients, per capability that can be exercised. This represents information about a single entity/device, which can provide multiple capabilities (with a different exercise cost for each).
type SingleEntityCapabilities e = Capabilities (ExerciseCost e)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After understanding the relationship between SingleEntityCapabilities and MultiEntityCapabilities, I was kind of expecting to see a function with a type like

Map e (SingleEntityCapabilities en) -> MultiEntityCapabilities e en

(that type might not work as written but hopefully you get the idea). Is there a function like this somewhere in a different module? Can it go in this module instead?

src/swarm-scenario/Swarm/Game/Device.hs Show resolved Hide resolved
src/swarm-scenario/Swarm/Game/Device.hs Outdated Show resolved Hide resolved
src/swarm-engine/Swarm/Game/Exception.hs Outdated Show resolved Hide resolved
src/Swarm/TUI/View.hs Outdated Show resolved Hide resolved
src/Swarm/TUI/View.hs Show resolved Hide resolved
@kostmo
Copy link
Member Author

kostmo commented Apr 25, 2024

All review comments now addressed, @byorgey.

@kostmo kostmo marked this pull request as ready for review April 25, 2024 06:07
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Apr 25, 2024
@mergify mergify bot merged commit f5ecd3f into main Apr 25, 2024
11 checks passed
@mergify mergify bot deleted the feature/capability-cost branch April 25, 2024 19:40
@kostmo kostmo added the CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. label Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. merge me Trigger the merge process of the Pull request.
Projects
None yet
2 participants