-
-
Notifications
You must be signed in to change notification settings - Fork 191
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: [#3157] return target on trigger #3176
Feat: [#3157] return target on trigger #3176
Conversation
There indeed is some hidden magic apologies, I've been meaning to fix up the sandbox to a modern build. I've got a half completed stale branch (https://github.com/excaliburjs/Excalibur/tree/chore/improve-sandbox-qol) The gist is you copy the built artifacts into the sandbox lib folder then run the build
|
@Autsider666 there are some flakey unit tests on main I haven't nailed down yet either :/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Autsider666 looks great!
38dd008
to
f02ef89
Compare
@eonarheim Updated the changelog, so PR is ready to be merged |
@Autsider666 Many thanks! |
===:clipboard: PR Checklist :clipboard:===
==================
Couldn't get the suggested setup to work without adding a lot of extra (code) complexity. I'd recommend dropping that requirement, because it's a lot easier to implement by the developers themselves by adding the check in their own
filter
callback to assure an entity is of a specific type (and useas ....
to placate TS if needed).I'll finish the last 2 points on the checklist after there's a green light on my current implementation.
Closes #3157
Changes:
target
is now 100% separate fromfilter
, because that's what the functionality described in the documentation implied (to me).Entity
andActor
interchangeably. Rewrote it to be 100% Entity (Although Entity doesn't have any collision mechanics build in, so an argument could be made to go for 100% Actor)