-
Notifications
You must be signed in to change notification settings - Fork 110
queueExecuteAndWrapIotas: allow specifying continuation #965
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
queueExecuteAndWrapIotas: allow specifying continuation #965
Conversation
Adds a defaulted queueExecuteAndWrapIotas() parameter to allow specifying the continuation (in addition to the generated FrameEvaluate frame). This should not change current behavior, and, due to @jvmoverloads, does not break compatibility with existing addons.
bfd1119 to
93b1c40
Compare
f0c4d14 to
4fd9ca4
Compare
| var continuation = SpellContinuation.Done.pushFrame(FrameEvaluate(SpellList.LList(0, iotas), false)) | ||
| // HACK: Ideally, we'd have a separate (SpellContinuation, ServerWorld) overload, but then 0.11.3 would cause mixins to break. Look into properly splitting this in 1.21? |
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.
Can you open an issue to track this?
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.
sure, what should it look like (or do you have examples)? not exactly sure what the issue conventions are
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.
Something along the lines of #926 would be fine. Just a reminder to clean up the code the next time we can make a breaking change.
object-Object
left a comment
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.
LGTM
|
seems to break hexxyinthealps and all mods that inject into this method weirdly, https://mclo.gs/rn9Glcl |
…" (fix #967) This reverts commit c64e5d7, reversing changes made to 22b7a6f. The changes made in #965 were meant to avoid any impact for addons, but we've since realized that adding an optional argument to a method still breaks mixins targeting that method. This caused a crash with HexxyInTheAlps and may also break other addons. While mixin impacts aren't technically breaking changes, we still don't really want to break an unknown number of addons this close to a release, so we'll resolve this issue by reverting #965 and revisiting this change at a later date.
Adds a defaulted queueExecuteAndWrapIotas() parameter to allow specifying the continuation (in addition to the generated
FrameEvaluateframe). This should not change current behavior, and, due to@JvmOverloads, does not break compatibility with existing addons.