Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003639DoomseekerUIpublic2019-04-30 21:112019-06-30 10:37
ReporterWubTheCaptain 
Assigned ToPol M 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version1.2 
Target Version1.3Fixed in Version1.3 
Summary0003639: Adding the path from Wadseeker's general settings to Doomseeker's File Paths hurts WAD tooltip performance
DescriptionDoomseeker's Configuration UI warns you in Wadseeker's General settings if the "Directory where Wadseeker will put WADs into" isn't found in Doomseeker's "File Paths" configuration.

But if you go and "fix" it as suggested by the UI warning, you'll shoot yourself in the foot and kill your performance when displaying servers' WAD list tooltips with "Tell me where my WADs are located" enabled.

What happens is Wadseeker's general settings always add the "put WADs into" path to the File Path list unconditionally. Do as the UI says and add it to File Paths, now you have it twice in Doomseeker's internal pathfinder's path list (invisible to the user).

I'm saying the UI is wrong or confusing with a major side-effect on performance, assuming the behavior to add "put WADs into" directory to pathfinder makes perfect sense.
Steps To ReproduceEnable "Tell me where my WADs are located" in File Paths settings.

Benchmark WAD tooltip delays with Pol M's code (see 0003625:0020562), with the target directory for Wadseeker in file (WAD) paths list and not. I noticed 10–80 ms difference (or more) in tooltip lag depending on how many files are missing when the path was also added to "File Paths", and that's a lot.
Additional Information0003625:0020578
Attached Filespng file icon 2019-04-30-205606_maim.png [^] (72,297 bytes) 2019-04-30 21:11

- Relationships
parent of 0003643resolvedPol M Unnecessary warning in Wadseeker general settings (when the "put WADs into" directory can't be found from File Paths list) 
related to 0003641resolvedWubTheCaptain First time configuration has the same path in File Paths as in Wadseeker's General settings the path to put the WADs into 
child of 0003625needs testingPol M Servers with lots of (big) WADs may take a long time to display the WADs column tooltip on server browser 

-  Notes
User avatar (0020589)
Zalewa (developer)
2019-05-01 10:29

Acknowledged. Things to do:

1. Remove the warning from the "Wadseeker" configuration box. User doesn't need to be alarmed as nothing bad is happening there.
2. Keep the feature where the Wadseeker path is auto-added to the "File Paths"
3. When passing paths to the seeker routines, ensure duplicates are removed.
4. Also, when a parent path is marked as 'recursive' and there's a sub-path of that 'recursive' path in the seeker routines, omit going into the sub-path explicitly as the 'recursive' search will already go there. To avoid depending on the filesystem to tell us if a directory is a sub-directory of a 'recursive' path this can be implemented naively by simply comparing the paths as
(back)slash-tokenized strings.
5. The configuration box should not attempt to remove sub-paths even if there's a 'recursive' parent-path defined. User can remove the 'recursive' parent-path and expect that the sub-path will stay there.
User avatar (0020594)
WubTheCaptain (developer)
2019-05-01 16:25
edited on: 2019-05-01 16:26

Yesterday I considered alternative approaches, but I don't think they make more sense than Zalewa's proposal above.
  • Merge the Wadseeker General settings dialog with File Paths. This doesn't make much sense, because Doomseeker and Wadseeker may be "independent" of each other.
  • Always display a copy of the Wadseeker "put WADs into" path in File Paths. This could lead to confusing UI and undefined behavior if the user would attempt to delete the path from File Paths dialog.


User avatar (0020595)
WubTheCaptain (developer)
2019-05-01 16:30

I'll break this into smaller child issues/subtasks, since the benefits are independent and easier to review that way.
User avatar (0020596)
WubTheCaptain (developer)
2019-05-01 16:43
edited on: 2019-05-01 16:46

Re 0003639:0020589:
  • 1 is 0003643.
  • 2 requires no action.
  • 3–4 are this ticket.
  • 5 I'm not sure, no action?


User avatar (0020603)
Zalewa (developer)
2019-05-01 20:51

5 is the same as 2 (no action required)
User avatar (0020656)
Pol M (developer)
2019-05-14 14:53

OK, I'll use the same pr for ticket 0003638
User avatar (0020679)
Pol M (developer)
2019-05-20 14:18

PR was merged.
User avatar (0020821)
Zalewa (developer)
2019-06-23 10:30

Found two problems:

1. The path deduplicator doesn't work when the same path with different slashes is used or when there's a trailing slash. These are considered as different paths:


  d:/wads
  d:/wads/
  d:\wads
  d:\wads\


2. When there's a recursive and a non-recursive duplication on the list, the recursive should be the one to always remain. Right now it appears that the path that remains is the one that is encountered first on the list.
User avatar (0020837)
Pol M (developer)
2019-06-25 17:41

PR
User avatar (0020838)
Zalewa (developer)
2019-06-25 21:05

Merged. I think path deduplication works correctly now but I'll put it in the testing status just in case someone else wants to verify.
User avatar (0020849)
Zalewa (developer)
2019-06-30 10:37

No one? Ok. Resolving.

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2019-04-30 21:11 WubTheCaptain New Issue
2019-04-30 21:11 WubTheCaptain File Added: 2019-04-30-205606_maim.png
2019-04-30 21:12 WubTheCaptain Relationship added child of 0003625
2019-04-30 21:16 WubTheCaptain Description Updated View Revisions
2019-04-30 21:23 WubTheCaptain Target Version => 1.3
2019-04-30 21:26 WubTheCaptain Description Updated View Revisions
2019-04-30 21:37 WubTheCaptain Note Added: 0020581
2019-04-30 21:37 WubTheCaptain Note Edited: 0020581 View Revisions
2019-04-30 21:38 WubTheCaptain Note Deleted: 0020581
2019-04-30 21:47 WubTheCaptain Relationship added related to 0003641
2019-05-01 10:29 Zalewa Note Added: 0020589
2019-05-01 10:29 Zalewa Status new => acknowledged
2019-05-01 13:25 Pol M Assigned To => Pol M
2019-05-01 13:25 Pol M Status acknowledged => assigned
2019-05-01 16:25 WubTheCaptain Note Added: 0020594
2019-05-01 16:26 WubTheCaptain Note Edited: 0020594 View Revisions
2019-05-01 16:30 WubTheCaptain Note Added: 0020595
2019-05-01 16:34 WubTheCaptain Relationship added parent of 0003643
2019-05-01 16:43 WubTheCaptain Note Added: 0020596
2019-05-01 16:44 WubTheCaptain Note Edited: 0020596 View Revisions
2019-05-01 16:44 WubTheCaptain Note Edited: 0020596 View Revisions
2019-05-01 16:46 WubTheCaptain Note Edited: 0020596 View Revisions
2019-05-01 20:51 Zalewa Note Added: 0020603
2019-05-14 14:53 Pol M Note Added: 0020656
2019-05-14 15:02 Pol M Status assigned => needs review
2019-05-20 14:18 Pol M Note Added: 0020679
2019-05-20 14:18 Pol M Status needs review => needs testing
2019-06-23 10:30 Zalewa Note Added: 0020821
2019-06-23 12:05 WubTheCaptain Status needs testing => confirmed
2019-06-25 16:07 Pol M Assigned To Pol M => WubTheCaptain
2019-06-25 16:07 Pol M Status confirmed => assigned
2019-06-25 16:07 Pol M Assigned To WubTheCaptain => Pol M
2019-06-25 17:41 Pol M Note Added: 0020837
2019-06-25 17:41 Pol M Status assigned => needs review
2019-06-25 21:05 Zalewa Note Added: 0020838
2019-06-25 21:05 Zalewa Status needs review => needs testing
2019-06-30 10:37 Zalewa Note Added: 0020849
2019-06-30 10:37 Zalewa Status needs testing => resolved
2019-06-30 10:37 Zalewa Fixed in Version => 1.3
2019-06-30 10:37 Zalewa Resolution open => fixed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2019 MantisBT Team
Powered by Mantis Bugtracker