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

Compositing Operations #522

Open
kpreid opened this issue Sep 20, 2024 · 3 comments
Open

Compositing Operations #522

kpreid opened this issue Sep 20, 2024 · 3 comments
Labels
area: simulation Things related to what can happen in the world as time progresses.

Comments

@kpreid
Copy link
Owner

kpreid commented Sep 20, 2024

We want it to be possible to use Modifier::Composite on blocks which both have tick_actions. This means:

  • Defining an algorithm for compositing operations — which might be fully general, or only support simple self-modifying cases like Operation::Become and Operation::StartMove.
  • Passing enough information through the Composite and CompositeOperator implementation to know how to execute the composition of operations, while still accounting for things like the reverse flag.
  • Making it possible to compose arbitrary tick_action schedules, which means that block::TickAction may need to represent multiple schedules and operations, and time::Schedule may need to be able to represent the union of any two schedules.
@kpreid kpreid added the area: simulation Things related to what can happen in the world as time progresses. label Sep 20, 2024
@kpreid
Copy link
Owner Author

kpreid commented Oct 14, 2024

This turns out to require rethinking of how all modifiers interact with operations. As I wrote in 2132094 which adds some inactive code that would try to implement compositing operations:

The big problem: this breaks move_inside_composite_destination(), because there are two potential semantics for modifiers acting on ops:

  1. modifiers modify the ops to stack on their effects, and
  2. modifiers are usually inert; the originally provided op applies to the whole block,

and Move kind of assumed 2 while our new Composite is (necessarily?) doing 1 in order to incorporate the source block.

I believe we need to switch everything to “case 1” and, in particular, Modifier::evaluate() should, instead of receiving block and this_modifier_index, receive a “view” of the original block which consists of the block without any of the current or following modifiers, which can then be built on by modifier evaluations. (Hm, how many reallocations does that demand?)

@kpreid
Copy link
Owner Author

kpreid commented Oct 14, 2024

Another example of “case 1” being the right choice: suppose a block with a Become operation has a Modifier::Inventory — surely the inventory should not be destroyed by default, so the inventory modifier must copy itself into the new block. (And in cases where the inventory should be destroyed, that can be expressed by its new InvInBlock parameters.)

kpreid added a commit that referenced this issue Dec 29, 2024
…lves.

This fixes the core problem of <#522>:
if we want modifiers in general to be able to modify a block’s actions,
then they must always build on top of the actions they are given by the
*previous* modifier.

There probably need to be further changes, but I’ll deal with those as
they come up.
@kpreid
Copy link
Owner Author

kpreid commented Dec 29, 2024

Commit d5b7ce7 switches over to “case 1” for Move and Composite.

  • Relevant ignored tests have been enabled.
  • This enabled e8c6cfb, unlockable doors in the dungeon.

However, the original goal of general operator composition is not yet achieved: we still need composable schedules and handling operators other than Become.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: simulation Things related to what can happen in the world as time progresses.
Projects
None yet
Development

No branches or pull requests

1 participant