Skip to content

Add eternascript submit_solution API#710

Merged
luxaritas merged 4 commits intodevfrom
feat/eternascript-submit
May 8, 2023
Merged

Add eternascript submit_solution API#710
luxaritas merged 4 commits intodevfrom
feat/eternascript-submit

Conversation

@luxaritas
Copy link
Copy Markdown
Member

@luxaritas luxaritas commented May 5, 2023

Summary

This provides script authors (eg, for the mutation/submission booster) a convenient and reliable way to submit solutions (without manually constructing a POST request, which is awkward and could result in incorrect validation or missing information (eg, the undoblock with the custom target not being included)

Implementation Notes

  • For multiple reasons, this has to be async. Promises seemed like the cleanest and most flexible way to do this.
  • This API distinguishes between "hard errors" (ie, required validation failed, the backend returned an error, etc) which actually reject the promise and "user cancellation" which causes the promise to resolve to false. This allows for script authors to handle these situations differently ("this errored" vs "the user was prompted and opted to bail" are semantically different)
  • There are two options that can be specified to adjust the behavior. The first is validate. If false, the validation steps that are user-bypassable (eg, "this constraint hasnt been satisfied") will be skipped. The other is "notifyOnError". If false, the promise will be immediately resolved with false instead of prompting if validate is true and validation fails, and also avoids surfacing informational dialogs that would normally occur if an error is returned from the backend (eg, for an in-use barcode). I have defaulted both of these to true, as that makes it less likely to result in undesired behavior if the options are not fully understood by the author (and is less likely to create confusion for the end-user).
  • submitCurrentPose, which is called by both submit_solution and as a callback for UI submission handling, is set to use the parameters required by the UI callbacks by default

Testing

Tested a variety of combinations of successful and unsuccessful submissions from the UI and via the Eternascript function (with various combinations of parameters)

Related Issues

Related discussion in https://forum.eternagame.org/t/is-there-a-working-bulk-submission-script/4552/

Copy link
Copy Markdown
Contributor

@tkaragianes tkaragianes left a comment

Choose a reason for hiding this comment

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

Seems all right, although the exploding conditionals to deal with error state isn't the easiest to track. Most of my concerns relate to adding comments to explain the intent behind the pipeline stages. As long as we document the submission API, I think it's fine from a script author DX perspective.

@tkaragianes
Copy link
Copy Markdown
Contributor

On further reflection, if the main use case is bulk submission of a large set of sequences, should the defaults be user intervention required? Like details = prompt and notifyOnError: true. I guess the argument would be that by default, we want to surface issues to the user, and put the onus on the script author to appropriately call the API in a bulk submission situation.

@luxaritas
Copy link
Copy Markdown
Member Author

luxaritas commented May 5, 2023

Yes, exactly - it's trivial for the author to use the correct parameters, but incorrectly-used defaults could lead to unintuitive behavior. In the case of details, they most likely would need to specify it anyways (this default just makes it work more reasonably if they don't), and I could forsee it being entirely possible that in the bulk submission case it may be desirable for either notifyOnError or validate to be enabled with the other disabled (but enabling both is "safe", and I'm not going to pretend to know which permutation would be desirable anyways)

@luxaritas luxaritas merged commit c29603f into dev May 8, 2023
@luxaritas luxaritas deleted the feat/eternascript-submit branch May 8, 2023 16:11
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.

2 participants