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

feat(std-contracts): add SystemCallbackComponent #303

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

dk1a
Copy link
Contributor

@dk1a dk1a commented Dec 20, 2022

FunctionComponent:

  • Stores an arbitrary contract's address and a selector to one of its functions.
  • You use staticcallFunctionSelector with your own arguments to that function.
  • If the contract address is a system, an outdated version of it could get called, which could make maintaining a FunctionComponent troublesome.

SystemCallbackComponent:

  • Stores a system ID and the arguments for that specific system's execute (note how no address is stored).
  • You use executeSystemCallback with your own IWorld address. You do NOT use your own arguments (although technically you could, but that seems like an antipattern).
  • Maintaining a SystemCallbackComponent gets troublesome only if the system's arguments change. Otherwise the system's logic can change seamlessly.
  • You do not need to know which system the callback calls, or what the arguments are (however you can inspect them if you want to).

This is designed primarily with Subsystems in mind, however it technically works with any systems and doesn't depend on #268

@dk1a dk1a requested a review from alvrs as a code owner December 20, 2022 23:42
@alvrs
Copy link
Member

alvrs commented Jan 5, 2023

I like this idea! For posterity, could you elaborate on a specific use case you have in mind?

@dk1a
Copy link
Contributor Author

dk1a commented Jan 5, 2023

About time to make https://github.com/dk1a/wanderer-cycle public, hard to explain some of my PRs without it. Do note it's still extremely unfinished.
I have ScopedDurationSubsystem and EffectSubsystem
Effect's executeApplyTimed calls Duration's executeIncrease and sets the onEnd callback to be Effect's executeRemove. Note how Duration is in another repo and is completely unaware of Effect. This lets me do real callbacks without any hardcoded circular dependencies

Copy link
Member

@alvrs alvrs left a comment

Choose a reason for hiding this comment

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

Great addition! 🚢

@alvrs alvrs merged commit 7b3a859 into latticexyz:main Jan 10, 2023
@holic
Copy link
Member

holic commented Jan 10, 2023

It'd be great to find an easy-to-understand use case that we can document alongside this!

@dk1a
Copy link
Contributor Author

dk1a commented Jan 10, 2023

@alvrs Oh, you merged it faster than I expected. Was gonna change it to account for #327. Do you want me to do a breaking change PR for this component soon-ish, or leave that for v2?

@alvrs
Copy link
Member

alvrs commented Jan 10, 2023

I assume you're referring to removing the requirement for an execute method? Since that's a breaking change anyway I'd leave it for v2. This PR seemed useful for v1 already though.

LPSCRYPT pushed a commit to LPSCRYPT/esp that referenced this pull request Jan 23, 2023
@dk1a dk1a deleted the dk1a/callback-component branch February 5, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants