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

raidboss: update arr and responses to use player objects #5891

Merged
merged 5 commits into from
Nov 4, 2023

Conversation

quisquous
Copy link
Owner

Followup to #5861.

@quisquous
Copy link
Owner Author

@valarnin A couple of things this brings up is:

  • should data.party.member(undefined) return ??? or should the trigger handle it explicitly (I am leaning towards making the trigger do this work but I guess I can see how awkward this becomes later)
  • should output support not only objects but arrays of objects and join them with commas into the final string (I'm not sure about this)

@valarnin
Copy link
Collaborator

valarnin commented Nov 2, 2023

  • should data.party.member(undefined) return ??? or should the trigger handle it explicitly (I am leaning towards making the trigger do this work but I guess I can see how awkward this becomes later)

I see two options here:

  1. data.party.member(undefined) returns undefined and the trigger handles it
  2. data.party.member(undefined) returns a dummy party member object with all fields set to ???

I'm fine with either one, honestly. The latter option would probably be better to reduce boilerplate in triggers though, and still be detectable if a trigger explicitly needed to check for it?

  • should output support not only objects but arrays of objects and join them with commas into the final string (I'm not sure about this)

I think there are a couple of other questions that would need answered first:

  1. Is , the most appropriate separator? Does that make sense for all the languages that cactbot supports, or would that need to be localize-able?
  2. Should the input string array be join'd directly and then substituted, or should each input string in the array be substituted and then the resultant array be joined, or possibly both with some sort of selector? e.g.:
const inputStrings = ['A', 'B', 'C'];
const outputString = 'something ${value}';

const option1Result = 'something A,B,C';
const option2Result = 'something A,something B,something C';

@quisquous
Copy link
Owner Author

quisquous commented Nov 2, 2023

  • should output support not only objects but arrays of objects and join them with commas into the final string (I'm not sure about this)

I think there are a couple of other questions that would need answered first:

  1. Is , the most appropriate separator? Does that make sense for all the languages that cactbot supports, or would that need to be localize-able?

As far as I can tell, this is generally what gets used, e.g. T6 Thorn Whip's twoTethers output string. There are some triggers (see: https://github.com/quisquous/cactbot/pull/5891/files#diff-7796360647af53a46da16de5fff30e73d36d01e87dfa0489475c60354678f905L45) that just insert commas directly (which imo they shouldn't do, but unless output starts returning non-string values, there's not much to be done).

A quick search for /map.*ShortName/ shows ~10 more examples of this (although other different uses of that as well).

I could see an argument that this is ~ok and if it's awkward in a particular language that trigger could handle it specially.

  1. Should the input string array be join'd directly and then substituted, or should each input string in the array be substituted and then the resultant array be joined, or possibly both with some sort of selector? e.g.:
const inputStrings = ['A', 'B', 'C'];
const outputString = 'something ${value}';

const option1Result = 'something A,B,C';
const option2Result = 'something A,something B,something C';

I think option1Result is the one that makes more sense. It's usually a "X on a, b, c" or "X with a, b, c".

quisquous added a commit that referenced this pull request Nov 3, 2023
Followup to #5861 and discussions on #5891.

As the comment in the code specifies, if a trigger passes in [player1, player2, player3] as a value, and a user specifies `${players.job}`, then return: `${players[0].job}, ${players[1].job}, ${players[2].job}`.

In general, this means that all array elements must either be simple strings/numbers or all share the same prop, or there will be errors below about non-existent properties. In practice, this likely will never happen.

This will also handle simpler cases like: `output.safeSpots({ dirs: [output.front!(), output.back!(), output.left!()] })`

This also allows for `data.party.member(undefined)`
to return '???', just to reduce boilerplate.
quisquous added a commit that referenced this pull request Nov 3, 2023
Followup to #5861 and discussions on #5891.

As the comment in the code specifies, if a trigger passes in `players:
[player1, player2, player3]`, and a user specifies `${players.job}`,
then return: `${players[0].job}, ${players[1].job}, ${players[2].job}`.

In general, this means that all array elements must either be simple
strings/numbers or all share the same prop, or there will be errors
below about non-existent properties. In practice, this likely will never
happen.

This will also handle simpler cases like: `output.safeSpots!({ dirs:
[output.front!(), output.back!(), output.left!()] })`

This also allows for `data.party.member(undefined)` to return '???',
just to reduce boilerplate.
github-actions bot pushed a commit that referenced this pull request Nov 3, 2023
#5895)

Followup to #5861 and discussions on #5891.

As the comment in the code specifies, if a trigger passes in `players:
[player1, player2, player3]`, and a user specifies `${players.job}`,
then return: `${players[0].job}, ${players[1].job}, ${players[2].job}`.

In general, this means that all array elements must either be simple
strings/numbers or all share the same prop, or there will be errors
below about non-existent properties. In practice, this likely will never
happen.

This will also handle simpler cases like: `output.safeSpots!({ dirs:
[output.front!(), output.back!(), output.left!()] })`

This also allows for `data.party.member(undefined)` to return '???',
just to reduce boilerplate. c3214ca
@quisquous quisquous merged commit d9304c6 into main Nov 4, 2023
@quisquous quisquous deleted the outputstrings_resources_arr branch November 4, 2023 00:29
github-actions bot pushed a commit that referenced this pull request Nov 4, 2023
github-actions bot pushed a commit that referenced this pull request Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants