Page MenuHomeWildfire Games

Fix mod list filter
ClosedPublic

Authored by Angen on Jun 1 2019, 10:28 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23269: Fix mod list filter
Summary

Currently when you apply filter in mod selection and tries to enable/disable some mod, you will not enable/disable the one you have selected (if you are lucky and the position of that mod is the same as not filtered you will but ... )

This patch is fixing this and assertion error when you have applied filter and you keep pressing down button and then enable mod.

Fix tooltip not refreshing when mod is enabled and another is selected among disabled mods.
Prevent moving enabled mods up and down (manual sorting) while filters are applied, because user would sort filtered mods and we need to sort not filtered version of them.

Test Plan

Test that mods enable, disable as should

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Angen created this revision.Jun 1 2019, 10:28 PM
Stan added a subscriber: Stan.Jun 1 2019, 10:32 PM
Stan added inline comments.
binaries/data/mods/mod/gui/modmod/modmod.js
218 ↗(On Diff #8278)

Why do the selection after displaying the list ?

Angen added inline comments.Jun 1 2019, 10:35 PM
binaries/data/mods/mod/gui/modmod/modmod.js
218 ↗(On Diff #8278)

displayModLists does filter action and sets what should be displayed so we need to do selection after we know what will be displayed to trigger event in cpp part

if we do this before, we can end up out of range

Stan added inline comments.Jun 1 2019, 10:37 PM
binaries/data/mods/mod/gui/modmod/modmod.js
218 ↗(On Diff #8278)

Might deserve a comment then ;)

Angen added a comment.Jun 1 2019, 10:38 PM

fixing problem in rP15677

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js
| 369| 369| 
| 370| 370| 	g_ModsEnabled.sort((folder1, folder2) =>
| 371| 371| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 372|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 372|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 373| 373| 
| 374| 374| 	displayModList("modsEnabledList", g_ModsEnabled);
| 375| 375| }
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1583/display/redirect

Angen added a reviewer: Restricted Owners Package.Jun 3 2019, 1:24 PM
Angen updated this revision to Diff 10526.Dec 8 2019, 9:43 AM
Angen edited the summary of this revision. (Show Details)

Fix tooltip not refreshing when mod is enabled and another is selected among disabled mods

Vulcan added a comment.Dec 8 2019, 9:46 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/712/display/redirect

Vulcan added a comment.Dec 8 2019, 9:57 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 368| 368| 
| 369| 369| 	g_ModsEnabled.sort((folder1, folder2) =>
| 370| 370| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 371|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 371|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 372| 372| 
| 373| 373| 	displayModList("modsEnabledList", g_ModsEnabled);
| 374| 374| }
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1228/display/redirect

Angen updated this revision to Diff 10527.Dec 8 2019, 10:06 AM
Angen retitled this revision from Fix mod list not enabling/disabling correct mod when applied filter to Fix mod list filter.
Angen edited the summary of this revision. (Show Details)

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/713/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 368| 368| 
| 369| 369| 	g_ModsEnabled.sort((folder1, folder2) =>
| 370| 370| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 371|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 371|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 372| 372| 
| 373| 373| 	g_ModsEnabledFiltered = displayModList("modsEnabledList", g_ModsEnabled);
| 374| 374| }
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1229/display/redirect

Angen planned changes to this revision.Dec 8 2019, 10:19 AM

so no, still able to break it

Angen updated this revision to Diff 10528.Dec 8 2019, 11:05 AM
Angen edited the summary of this revision. (Show Details)

Prevent moving enabled mods up and down (manual sorting) while filters are applied, because user would sort filtered mods and we need to sort not filtered version of them.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/714/display/redirect

Angen updated this revision to Diff 10529.Dec 8 2019, 11:07 AM

white spaces

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 281| 281| 	// to not filtered positions so changes will persist.
| 282| 282| 	if (areFilters())
| 283| 283| 		return;
| 284|    |-	
|    | 284|+
| 285| 285| 	let obj = Engine.GetGUIObjectByName(objectName);
| 286| 286| 	let idx = obj.selected;
| 287| 287| 	if (idx == -1)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 294| 294| 
| 295| 295| 	let tmp = g_ModsEnabled[idx];
| 296| 296| 	g_ModsEnabled[idx] = g_ModsEnabled[idx2];
| 297|    |-	g_ModsEnabled[idx2] = tmp;	
|    | 297|+	g_ModsEnabled[idx2] = tmp;
| 298| 298| 	
| 299| 299| 	g_ModsEnabledFiltered = displayModList("modsEnabledList", g_ModsEnabled);
| 300| 300| 	obj.selected = idx2;
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 295| 295| 	let tmp = g_ModsEnabled[idx];
| 296| 296| 	g_ModsEnabled[idx] = g_ModsEnabled[idx2];
| 297| 297| 	g_ModsEnabled[idx2] = tmp;	
| 298|    |-	
|    | 298|+
| 299| 299| 	g_ModsEnabledFiltered = displayModList("modsEnabledList", g_ModsEnabled);
| 300| 300| 	obj.selected = idx2;
| 301| 301| }
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 378| 378| 
| 379| 379| 	g_ModsEnabled.sort((folder1, folder2) =>
| 380| 380| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 381|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 381|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 382| 382| 
| 383| 383| 	g_ModsEnabledFiltered = displayModList("modsEnabledList", g_ModsEnabled);
| 384| 384| }
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1230/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/715/display/redirect

Angen added a comment.Dec 8 2019, 11:11 AM

some notes

binaries/data/mods/mod/gui/modmod/modmod.js
206 ↗(On Diff #10529)

this discards current sorting of enabled mods done by user

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 378| 378| 
| 379| 379| 	g_ModsEnabled.sort((folder1, folder2) =>
| 380| 380| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 381|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 381|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 382| 382| 
| 383| 383| 	g_ModsEnabledFiltered = displayModList("modsEnabledList", g_ModsEnabled);
| 384| 384| }
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1231/display/redirect

Angen updated this revision to Diff 10531.Dec 8 2019, 11:27 AM

fix last note

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/716/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
|    |++++| /zpool0/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
| 381| 381| 
| 382| 382| 	g_ModsEnabled.sort((folder1, folder2) =>
| 383| 383| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 384|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 384|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 385| 385| 
| 386| 386| 	g_ModsEnabledFiltered = displayModList("modsEnabledList", g_ModsEnabled);
| 387| 387| }
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1232/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Dec 20 2019, 9:19 PM
Closed by commit rP23269: Fix mod list filter (authored by Angen). · Explain Why
This revision was automatically updated to reflect the committed changes.