Pet Battle Scripts

Pet Battle Scripts

265k Downloads

REMATCH_TEAM_OVERWRITTEN inconstancy with how scripts are managed.

gizzmo opened this issue ยท 7 comments

commented

Rematch.events:Register(self, 'REMATCH_TEAM_OVERWRITTEN', function(self, newTeamID, oldTeamID)

This is what i was worried about. After some testing i found some inconstancy with how things are handled, with scripts moving when teams are overwritten. I don't know if its our implementation, or an issue with how Rematch is firing/using the event.

Scenario 1:

If the old team has a script, when the new team overwrites it, the script still exists. That's ok i guess. The user can just delete the script.

Steps:

  • Create a team named Team 1 and add a script test(This is Team 1)
  • Import/load random pets (edit: Save and overwrite Team 1)
  • Script still exists.

Scenario 2:

If the old team does not have a script, and the new team does, the script moves. As expected. ๐Ÿ‘๐Ÿป

Steps:

  • Create two teams (Team 1/Team 2)
  • Add script to "Team 2" with script test(Team 2)
  • Edit "Team 2" , change name to "Team 1" and overwrite existing team
  • Team 1 now has a script with the name "Team 1" and contents test(Team 2)

Scenario 3:

If both teams have scripts, the new teams script does not overwrite old team's script. The old team's script should be deleted.

Steps:

  • Create two teams (Team 1/Team 2)
  • Add a script to to both of them, with contents that's easily tracked: test(This is Team 1)`test(This is Team 2)`
  • Edit "Team 2" and change name to "Team 1" and overwrite the existing team.
  • "Team 1" keeps the scripted named "Team 1" with test(This is Team 1).
  • "Team 1" should have the script test(This is Team 2) script.

Scenario 4:

Steps:

  • Create two teams with easily trackable scripts.
  • Load Team 1. (you can change pets to make it easier to track, just as long as Team 1 remains the loaded team)
  • Click "Save As" name it "Team 2", and overwrite existing team.
  • Team 1 remains, but has script named "Team 2" with contents test(Team 2)
  • Team 2 has no script.

Team 1 shouldn't have been touched, and Team 2 should either maintain the old Team 2 script, or have no script.

commented

#64 now implements that give-up behavior:

  • scenario 1: script is gone
  • scenario 2: script is named team1 and has test(team2)
  • scenario 3: script is named team1 and has test(team2)
  • scenario 4: script for team1 is unchanged, script for team2 is gone.
commented

After so many more testing. I'm thinking we just not use this event at all, unless Gello does something to alleviate this issue. Without the event, most things happen in a acceptable manner. The only weird one is overwriting with a copy (save as).

How often are users even overwriting existing teams with other existing teams? For me overwrites happen more with an import, than renaming a team.

I suggest a true "give-up" and just add a warning to the Overwrite Dialog,

self:SecureHook(Rematch.dialog:GetDialogInfo('NameConflict'), 'refreshFunc', function(dialog)
    local text = dialog.Text.Text:GetText()

    dialog.Text:SetText(text.."\n\n".. Rematch.constants.HEX_RED .."PetBattleScripts:|r\nOverwriting teams can have unintended loss of scripts. We recommend backing up scripts before doing this.")
end)

2023 11 03 11 07 07 735

commented

How often are users even overwriting existing teams with other existing teams? For me overwrites happen more with an import, than renaming a team.

Yeah, I doubt this pretty much ever happens outside of importing. And with importing, I would expect the script to be gone anyway (the last team and script didn't work, so next!)

I don't really think we need to have a warning. We don't warn on deletion either, so why here? You're overwriting a team, and people see our scripts as part of their Rematch teams already anyway.

commented

I think i figured it out. Found access to know if this event is an edit or a save-as. Overwrites seem to act like they should.

Rematch.events:Register(self, 'REMATCH_TEAM_OVERWRITTEN', function(self, teamID, overwriteID)
    -- teamID is being overwritten with import, and wont have a script
    if teamID and not overwriteID then
        return self:RemoveScript(teamID)
    end

    local subject = Rematch.dialog:GetSubject()
    if subject and subject.saveMode ~= Rematch.constants.SAVE_MODE_EDIT then
        if teamID and self:GetScript(teamID) then -- team exists, and has script
            self:CopyScript(teamID, overwriteID)

        else -- teamID doesnt exist or teamID has no script
            self:RemoveScript(overwriteID)
        end
    end
    -- if SAVE_MODE_EDIT, no action is needed, REMATCH_TEAM_DELETED will delete overwriteID's script
end)

(thanks wowlua)

EDIT: Cleaned up code.

Bonus question, do you want to copy the script when you duplicate and overwrite a team (save-as)? Im leaning to not copy, as when you save-as and create a new team, there's no way i found to copy a script onto that team.

commented

Bonus question, do you want to copy the script when you duplicate and overwrite a team (save-as)? Im leaning to not copy, as when you save-as and create a new team, there's no way i found to copy a script onto that team.

Thinking in terms of "people only know teams", I'd probably copy the script as well if there is a copy of the team made.

commented

I mentioned this confusion on the Rematch thread a while ago:

https://www.warcraftpets.com/community/forum/viewtopic.php?f=19&t=20340&start=40#p149486

commented

Scenario 1

  • Fires OVERWRITTEN(team:1), UPDATED(team:1,nil,nil).
  • Letting OVERWRITTEN(just-one-argument) remove the script would be fine.

Scenario 2

  • I get the inverse behavior: the script vanishes. For me, Rematch uses team:2 in the end, which makes sense.
  • Fires OVERWRITTEN(new=team:1, old=team:2), DELETED(team:1,nil,nil), UPDATED(team:2,nil,nil).
  • I have interpreted new=team:1, old=team:2 as "team:2 is gone and there now is team:1 only", but obviously it is intended to be "team:1 is gone because of team:2". Since I do that, I move script for team:2 to team:1, then react to DELETED and get rid of it.
  • Letting OVERWRITTEN(two-arguments) do nothing would be fine.

Scenario 3

  • Fires OVERWRITTEN(new=team:1, old=team:2), DELETED(team:1), UPDATED(team:2).
  • Again, I first move script for team:2 to team:1, then remove it as wrongly assumed. Since the team still uses team:2, not reacting at all would have been fine, again.

Scenario 4

  • Fires OVERWRITTEN(new=team:2, old=team:1), UPDATED(team:2).
  • Once again, I move the script from team:1 to team:2. It is removed from team1 since I interpret it as a move, not a copy.
  • Ignoring the event would not work this time, shame.

???

To me it seems that the events are very hard to interpret correctly, and they behave differently depending on situation. Maybe it is correct to interpret OVERWRITTEN(team, ignore) as "remove script for team" and nothing else?