Skip to content

Commit

Permalink
CATROID-1056 Fix wrong variable references after undo
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstoeg
mstoeg committed Apr 23, 2021
1 parent ba17309 commit 46f369b
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 29 deletions.
21 changes: 14 additions & 7 deletions ...androidTest/java/org/catrobat/catroid/uiespresso/formulaeditor/FormulaEditorUndoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@
import static org.catrobat.catroid.uiespresso.content.brick.utils.ColorPickerInteractionWrapper.onColorPickerPresetButton;
import static org.catrobat.catroid.uiespresso.formulaeditor.utils.FormulaEditorDataListWrapper.onDataList;
import static org.catrobat.catroid.uiespresso.formulaeditor.utils.FormulaEditorWrapper.onFormulaEditor;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

import static androidx.test.espresso.Espresso.closeSoftKeyboard;
import static androidx.test.espresso.Espresso.onView;
Expand All @@ -72,6 +72,7 @@ public class FormulaEditorUndoTest {
private static final String NEW_VARIABLE_NAME = "NewVariable";
private static final int VARIABLE_VALUE = 5;
private static final String NEW_VARIABLE_VALUE = "10";
UserVariable userVariable;

@Rule
public FragmentActivityTestRule<SpriteActivity> baseActivityTestRule = new
Expand All @@ -87,7 +88,7 @@ public void setUp() throws Exception {
brickPosition = 1;
Script script = BrickTestUtils.createProjectAndGetStartScript(FormulaEditorUndoTest.class.getName());
script.addBrick(new PlaceAtBrick());
UserVariable userVariable = new UserVariable(VARIABLE_NAME, VARIABLE_VALUE);
userVariable = new UserVariable(VARIABLE_NAME, VARIABLE_VALUE);
ProjectManager.getInstance().getCurrentProject().addUserVariable(userVariable);
script.addBrick(new SetVariableBrick(new Formula(0), userVariable));
baseActivityTestRule.launchActivity();
Expand Down Expand Up @@ -280,7 +281,8 @@ public void testUndoFormulaDeleteVariable() {

assertNotNull(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME));

assertTrue(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue().equals(VARIABLE_VALUE));
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME), userVariable);
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue(), VARIABLE_VALUE);
}

@Category({Cat.AppUi.class, Level.Smoke.class})
Expand Down Expand Up @@ -309,16 +311,19 @@ public void testUndoFormulaRenameVariable() {

assertNull(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME));
assertNotNull(ProjectManager.getInstance().getCurrentProject().getUserVariable(NEW_VARIABLE_NAME));
assertTrue(ProjectManager.getInstance().getCurrentProject().getUserVariable(NEW_VARIABLE_NAME).getValue().equals(VARIABLE_VALUE));
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(NEW_VARIABLE_NAME), userVariable);
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(NEW_VARIABLE_NAME).getValue(), VARIABLE_VALUE);

onView(withId(R.id.menu_undo))
.perform(click());
userVariable.setName(VARIABLE_NAME);
onView(withId(R.id.menu_undo))
.check(doesNotExist());

assertNull(ProjectManager.getInstance().getCurrentProject().getUserVariable(NEW_VARIABLE_NAME));
assertNotNull(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME));
assertTrue(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue().equals(VARIABLE_VALUE));
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME), userVariable);
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue(), VARIABLE_VALUE);
}

@Category({Cat.AppUi.class, Level.Smoke.class})
Expand All @@ -345,14 +350,16 @@ public void testUndoFormulaEditVariable() {
onView(withId(R.id.menu_undo))
.check(matches(isDisplayed()));

assertTrue(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue().equals(NEW_VARIABLE_VALUE));
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME), userVariable);
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue(), NEW_VARIABLE_VALUE);

onView(withId(R.id.menu_undo))
.perform(click());
onView(withId(R.id.menu_undo))
.check(doesNotExist());

assertTrue(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue().equals(VARIABLE_VALUE));
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME), userVariable);
assertEquals(ProjectManager.getInstance().getCurrentProject().getUserVariable(VARIABLE_NAME).getValue(), VARIABLE_VALUE);
}

@Category({Cat.AppUi.class, Level.Smoke.class})
Expand Down
28 changes: 18 additions & 10 deletions catroid/src/main/java/org/catrobat/catroid/content/Project.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,24 @@ public List<UserVariable> getUserVariablesCopy() {
return userVariablesCopy;
}

public void setUserVariables(List<UserVariable> newUserVariables) {
userVariables = newUserVariables;
public <T> void restoreUserDataValues(List<T> currentUserDataList, List<T> userDataListToRestore) {
for (T userData : currentUserDataList) {
for (T userDataToRestore : userDataListToRestore) {
if (userData.getClass() == UserVariable.class) {
UserVariable userVariable = (UserVariable) userData;
UserVariable newUserVariable = (UserVariable) userDataToRestore;
if (userVariable.getName().equals(newUserVariable.getName())) {
userVariable.setValue(newUserVariable.getValue());
}
} else {
UserList userList = (UserList) userData;
UserList newUserList = (UserList) userDataToRestore;
if (userList.getName().equals(newUserList.getName())) {
userList.setValue(newUserList.getValue());
}
}
}
}
}

public UserVariable getUserVariable(String name) {
Expand Down Expand Up @@ -258,10 +274,6 @@ public List<UserList> getUserLists() {
return userLists;
}

public void setUserLists(List<UserList> newUserLists) {
userLists = newUserLists;
}

public List<UserList> getUserListsCopy() {
if (userLists == null) {
userLists = new ArrayList<>();
Expand Down Expand Up @@ -297,10 +309,6 @@ public boolean removeUserList(String name) {
return false;
}

public void setMultiplayerVariables(List<UserVariable> newMultiplayerVariables) {
multiplayerVariables = newMultiplayerVariables;
}

public List<UserVariable> getMultiplayerVariablesCopy() {
if (multiplayerVariables == null) {
multiplayerVariables = new ArrayList<>();
Expand Down
24 changes: 18 additions & 6 deletions catroid/src/main/java/org/catrobat/catroid/content/Sprite.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,24 @@ public <T> boolean checkUserData(T newUserData, List<T> oldUserData) {
return false;
}

public void setUserVariables(List<UserVariable> newUserVariables) {
userVariables = newUserVariables;
public <T> void restoreUserDataValues(List<T> currentUserDataList, List<T> userDataListToRestore) {
for (T userData : currentUserDataList) {
for (T userDataToRestore : userDataListToRestore) {
if (userData.getClass() == UserVariable.class) {
UserVariable userVariable = (UserVariable) userData;
UserVariable newUserVariable = (UserVariable) userDataToRestore;
if (userVariable.getName().equals(newUserVariable.getName())) {
userVariable.setValue(newUserVariable.getValue());
}
} else {
UserList userList = (UserList) userData;
UserList newUserList = (UserList) userDataToRestore;
if (userList.getName().equals(newUserList.getName())) {
userList.setValue(newUserList.getValue());
}
}
}
}
}

public List<UserVariable> getUserVariablesCopy() {
Expand Down Expand Up @@ -262,10 +278,6 @@ public boolean addUserVariable(UserVariable userVariable) {
return userVariables.add(userVariable);
}

public void setUserLists(List<UserList> newUserLists) {
userLists = newUserLists;
}

public List<UserList> getUserListsCopy() {
if (userLists == null) {
userLists = new ArrayList<>();
Expand Down
14 changes: 8 additions & 6 deletions catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ScriptFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,9 @@ public void loadProjectAfterUndoOption() {
@Override
public void onLoadFinished(boolean success) {
ProjectManager.getInstance().setCurrentSceneAndSprite(currentSceneName, currentSpriteName);
loadVariables();
if (checkVariables()) {
loadVariables();
}
refreshFragmentAfterUndo();
}

Expand Down Expand Up @@ -890,11 +892,11 @@ private void loadVariables() {
Sprite currentSprite = projectManager.getCurrentSprite();
Project project = projectManager.getCurrentProject();

project.setUserVariables(savedUserVariables);
project.setMultiplayerVariables(savedMultiplayerVariables);
project.setUserLists(savedUserLists);
currentSprite.setUserVariables(savedLocalUserVariables);
currentSprite.setUserLists(savedLocalLists);
project.restoreUserDataValues(project.getUserVariables(), savedUserVariables);
project.restoreUserDataValues(project.getMultiplayerVariables(), savedMultiplayerVariables);
project.restoreUserDataValues(project.getUserLists(), savedUserLists);
currentSprite.restoreUserDataValues(currentSprite.getUserVariables(), savedLocalUserVariables);
currentSprite.restoreUserDataValues(currentSprite.getUserLists(), savedLocalLists);
}

private void refreshFragmentAfterUndo() {
Expand Down

0 comments on commit 46f369b

Please sign in to comment.