REMATCH_TEAM_OVERWRITTEN inconstancy with how scripts are managed.
gizzmo opened this issue ยท 7 comments
Line 75 in 88e719b
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.
#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.
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)
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.
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.
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.
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
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 isteam:1
only", but obviously it is intended to be "team:1
is gone because ofteam:2
". Since I do that, I move script forteam:2
toteam:1
, then react toDELETED
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
toteam:1
, then remove it as wrongly assumed. Since the team still usesteam: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?