MantisBT - Doomseeker
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0003639DoomseekerUIpublic2019-04-30 21:112019-07-30 10:13
ReporterWubTheCaptain 
Assigned ToPol M 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
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
TagsNo tags attached.
Relationships
parent of 0003643closed Pol M Unnecessary warning in Wadseeker general settings (when the "put WADs into" directory can't be found from File Paths list) 
related to 0003641closed WubTheCaptain 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 0003625closed Pol M Servers with lots of (big) WADs may take a long time to display the WADs column tooltip on server browser 
Attached Filespng 2019-04-30-205606_maim.png (72,297) 2019-04-30 21:11
https://zandronum.com/tracker/file_download.php?file_id=2470&type=bug
png
Issue History
Date ModifiedUsernameFieldChange
2019-04-30 21:11WubTheCaptainNew Issue
2019-04-30 21:11WubTheCaptainFile Added: 2019-04-30-205606_maim.png
2019-04-30 21:12WubTheCaptainRelationship addedchild of 0003625
2019-04-30 21:16WubTheCaptainDescription Updatedbug_revision_view_page.php?rev_id=12525#r12525
2019-04-30 21:23WubTheCaptainTarget Version => 1.3
2019-04-30 21:26WubTheCaptainDescription Updatedbug_revision_view_page.php?rev_id=12528#r12528
2019-04-30 21:37WubTheCaptainNote Added: 0020581
2019-04-30 21:37WubTheCaptainNote Edited: 0020581bug_revision_view_page.php?rev_id=12530
2019-04-30 21:38WubTheCaptainNote Deleted: 0020581
2019-04-30 21:47WubTheCaptainRelationship addedrelated to 0003641
2019-05-01 10:29ZalewaNote Added: 0020589
2019-05-01 10:29ZalewaStatusnew => acknowledged
2019-05-01 13:25Pol MAssigned To => Pol M
2019-05-01 13:25Pol MStatusacknowledged => assigned
2019-05-01 16:25WubTheCaptainNote Added: 0020594
2019-05-01 16:26WubTheCaptainNote Edited: 0020594bug_revision_view_page.php?bugnote_id=20594#r12543
2019-05-01 16:30WubTheCaptainNote Added: 0020595
2019-05-01 16:34WubTheCaptainRelationship addedparent of 0003643
2019-05-01 16:43WubTheCaptainNote Added: 0020596
2019-05-01 16:44WubTheCaptainNote Edited: 0020596bug_revision_view_page.php?bugnote_id=20596#r12545
2019-05-01 16:44WubTheCaptainNote Edited: 0020596bug_revision_view_page.php?bugnote_id=20596#r12546
2019-05-01 16:46WubTheCaptainNote Edited: 0020596bug_revision_view_page.php?bugnote_id=20596#r12547
2019-05-01 20:51ZalewaNote Added: 0020603
2019-05-14 14:53Pol MNote Added: 0020656
2019-05-14 15:02Pol MStatusassigned => needs review
2019-05-20 14:18Pol MNote Added: 0020679
2019-05-20 14:18Pol MStatusneeds review => needs testing
2019-06-23 10:30ZalewaNote Added: 0020821
2019-06-23 12:05WubTheCaptainStatusneeds testing => confirmed
2019-06-25 16:07Pol MAssigned ToPol M => WubTheCaptain
2019-06-25 16:07Pol MStatusconfirmed => assigned
2019-06-25 16:07Pol MAssigned ToWubTheCaptain => Pol M
2019-06-25 17:41Pol MNote Added: 0020837
2019-06-25 17:41Pol MStatusassigned => needs review
2019-06-25 21:05ZalewaNote Added: 0020838
2019-06-25 21:05ZalewaStatusneeds review => needs testing
2019-06-30 10:37ZalewaNote Added: 0020849
2019-06-30 10:37ZalewaStatusneeds testing => resolved
2019-06-30 10:37ZalewaFixed in Version => 1.3
2019-06-30 10:37ZalewaResolutionopen => fixed
2019-07-30 10:13WubTheCaptainStatusresolved => closed

Notes
(0020589)
Zalewa   
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.
(0020594)
WubTheCaptain   
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.
(0020595)
WubTheCaptain   
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.
(0020596)
WubTheCaptain   
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?
(0020603)
Zalewa   
2019-05-01 20:51   
5 is the same as 2 (no action required)
(0020656)
Pol M   
2019-05-14 14:53   
OK, I'll use the same pr for ticket 0003638
(0020679)
Pol M   
2019-05-20 14:18   
PR was merged.
(0020821)
Zalewa   
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.
(0020837)
Pol M   
2019-06-25 17:41   
PR
(0020838)
Zalewa   
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.
(0020849)
Zalewa   
2019-06-30 10:37   
No one? Ok. Resolving.