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: Default character rewards in prompts #1087

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

CH3RVB
Copy link

@CH3RVB CH3RVB commented Oct 14, 2024

-Add default prompt rewards for characters. (Surprised this wasn't a thing already.)
-Rewards are handled with a flag that determines if they are the focus of the prompt (multiple can be designated if need be, but this prevents submissions handing out rewards automatically to every tagged character even if they're just a small cameo or whatever)
-Did something similar to prompts where submission of the prompt adds the rewards to the asset list, so those can be edited as well, like user rewards they will be removed upon cancellation so no duplication occurs

Ty to @ScuffedNewt for handing me the loot_select prefixes so two loot_selects can exist on the same page, and also for the is_focus flag (going to PR a check on the claymores ext so no possible migration issues can occur)

Let me know if i missed anything weird here or went about it wrong HAHAHAH my brain is a little on the fritz rn

@itinerare itinerare added enhancement New feature or request needs review Pull requests that are pending community review labels Oct 14, 2024
Copy link
Contributor

@ScuffedNewt ScuffedNewt left a comment

Choose a reason for hiding this comment

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

minor comments and some stuff for my own understanding!

@@ -79,7 +79,14 @@ public function category() {
* Get the rewards attached to this prompt.
*/
public function rewards() {
return $this->hasMany(PromptReward::class, 'prompt_id');
return $this->hasMany(PromptReward::class, 'prompt_id')->where('earner_type', 'User');
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of earner_type can this just be type?

Copy link
Author

Choose a reason for hiding this comment

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

ABSOLUTELY i wasn't sure if type would be too vague or not but i can swap it out

* @param mixed $submission the submission object
* @param mixed $character
*/
private function removeCharacterPromptAttachments($submission, $character) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain this function some more? its not very descriptive from the comment, curious because of how its being stored on the SubmissionCharacter data

Copy link
Author

@CH3RVB CH3RVB Oct 14, 2024

Choose a reason for hiding this comment

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

tbh it was pretty new to me too before i started taking a closer look, since submissions work on drafts in 3.0/dev, it had a new function added to help with that for users which i took for characters and edited for their purpose

this is basically how it goes:

  • if submission_status == pending, add the default prompt rewards to the submission, and merge those with anything the user added as well, so that the admins can potentially edit the rewards instead of granting them immediately...
  • however when a draft is cancelled, then resubmitted, basically it would "duplicate" any of the default rewards when resubmitted.. not unfixable, but inconvenient
  • removePromptAttachments() would take the rewards of a cancelled draft, and remove any of the default rewards from it, so that when the user goes to edit it again, there would be no duplication of the default rewards

i was wondering how drafts handled how that worked without causing havoc but apparently that was it HAHAH i'm probably not explaining it right...
if you take out the removeAsset() parts of removePromptAttachments() you can see what i mean firsthand with user-specific rewards duplicating

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yes i think ic... LGTM in that case! ill approve tomorrow after another quick look

@@ -116,6 +123,7 @@
@section('scripts')
@parent
@include('js._loot_js', ['showLootTables' => true, 'showRaffles' => true])
@include('js._loot_js', ['showLootTables' => true, 'showRaffles' => true, 'prefix' => 'character_', 'isCharacter' => true])
Copy link
Contributor

Choose a reason for hiding this comment

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

yay you used my prefix thing :D

@SpeedyD
Copy link
Contributor

SpeedyD commented Oct 28, 2024

I was actually wondering.. if a user adds someone else's character to the prompt for rewards, does the other user, whose character is now apparently getting a reward, get notified on this reward?

Maybe I'm missing something, but I went over the code and I can't find this happening.. 🤔.. Wouldn't be surprised if I simply looked over it or failed to notice it or something.. 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs review Pull requests that are pending community review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants