Use not equal comparsion for timestamps #25

Ortham · 2015-11-23T09:41:08Z

Currently timetamp comparisons check if the file timetamp is newer than the cached timestamp, but it is possible for the timestamp of a file to be changed so that it is older, as identified by @Utumno in wrye-bash/wrye-bash#195.

The simplest fix for this is to use a not equals comparison operator rather than a greater than operator.

Should close for good wrye-bash/wrye-bash#195

Ortham · 2015-11-23T18:01:53Z

Re-opening this as I think there may be a problem with the fix implemented.

One timestamp is stored, and it is set to the timestamp of the last file written, but it's possible for the plugin folder to have an older timestamp if the last file was written a second or more after the last plugin directory change.

Checking for changes in such a situation will therefore see a plugin folder that has a different timestamp and so will see it as having changed since the last save. I need to think how to turn this into a test...

This is a follow-up fix to the fix for #25. False positives could be caused when two data sources (eg. plugins folder and active plugins file) have different timestamps before data is loaded, or when a file is changed but no plugins folder changes are made.

libloadorder has several flaws, both in its API and its internal design: - from a design point of view it is built around a LoadOrder class (and before refactoring, also an ActivePlugins class). This leads to an ifelseheimer which only gets worse as new games are added, never mind it is unintuitive. It should be built around a Games classes hierarchy. This anti-OO design decision made me finally give up trying to patch upstream. - API: load order and active plugins handling should go together. With fallout4 this is made obvious. To me however, (as the main Bash maintainer) this has always been obvious - most operations need passing info on both load order and active plugins. The set/get active/load_order orthogonal API is simply inadequate for most but the simplest operations. There were other issues of equal importance: Bash features sophisticated file caches in the FileInfos > ModInfos hierarchy and a full machinery of record definitions. So having liboadorder recalculate esm status and validate plugins was thrown away performance and there was no work around for that - heck even calculating modification times and sizes of plugins was not needed. The API should expose switches to disable validation and should cache the plugins info. At the end of the day it should accept maps with sizes and mod times - recalculating those is system calls - so _measurable_ performance (and inelegant, which to me is a bigger annoyance). Another API issue was the caches - only recently fixed to check for different modification time as opposed to greater modification time, see: Utumno/libloadorder@d40f825 Ortham/libloadorder#25 (this has costed me a couple of months of debugging on the python side, trying to fix #195) The API should allow forcing a reload of load order in the case (for instance) of an esm flip, and in general whenever the client sees fit. Yet another issue was the automatic rewriting of plugins and loadorder txts - what if I wanted to warn the user or back them up ? The list goes on and on, but there is also another important point: *complexity*. Having this inflexible API to work with I had to monkey patch the python side, making the code very difficult to reason about - a casual example was validity checks (see Utumno/libloadorder#6) but that's just an obvious tip of a multiheaded iceberg - I basically had to dig into the C++ code and spend hours reasoning on what to expect in python and how to workaround it for my purposes. Not funny. I have elaborated on those points in a countless number of posts, issues and commit messages and comments - but most importantly in: https://github.com/Utumno/libloadorder/issues?utf8=%E2%9C%93&q= With the advent of the new FO4 star load order and with the rest of refactoring reaching a point where this could be addressed, I finally decided to solve this problem for good. My design is based on my several years of experience programming the most elaborate mod manager out there, so it is impossible to convey in a commit message. The sketching of the issues above is far from complete, but should give you a glimpse of the situation. The solution is an original API featuring three levels of abstraction: - a Games classes hierarchy that takes care of the lower level of load order handling. It leverages the modInfos singleton that contains full information on plugins state and exports a get and set load order API that is finally atomic re load order and active plugins management. The API also consists of methods that take care of timestamp load order peculiarities and report on the changed state of the txts involved, letting however the client decide if the cached info is to be discarded. - load_order.py that takes care of caching the load order (and soon of locking it). Separation of concerns. - the ModInfos API for the rest of Bash to use. Now it is clear when we query our cache (via load_order.py API) as opposed to when we modify or update it (ModInfos API) and refresh of our data structures is taken care in a single point (the @_lo_cache decorator) eliminating refresh glitches and most importantly business logic code from basher/ (ie the UI). The intermediate Plugins class is finally eradicated. It is interesting to see how the API evolves from lower to higher level - the ModInfos API indeed thinks in terms of active plugins and load order, while the underlying operations are atomic. The levels of abstraction introduced permit me to elegantly solve issues that are marked wontfix in libloadorder such as saving the txts only when needed: Ortham/libloadorder#29 That is not only a performance gain - it is the correct thing to do. The API is still not frozen, in particular stuff may move between games.py and load_order.py (which needs PEP8'ing), but the merge is fully functional. To stress test the API I went ahead and implemented #256 - has rough edges still but should function in most usual cases as expected. The commits in this merge belong together and they may introduce minor bugs but the net number of bugs fixed - design or otherwise - certainly outnumbers the presumptive regressions. Plus now I need only fix the python side I am familiar with (as I wrote it). This merge also contains some work on mergeability checks and the usual refactoring. Closes #295. Signed-off-by: MrD <[email protected]>

Ortham added the bug label Nov 23, 2015

Ortham closed this as completed in ab2b3fc Nov 23, 2015

Utumno referenced this issue in Utumno/libloadorder Nov 23, 2015

[7.5.2]Fix for random behavior when swapping plugin.txt:

d40f825

Should close for good wrye-bash/wrye-bash#195

Ortham mentioned this issue Nov 23, 2015

fix_plugin_lists misses some fixes #16

Closed

Ortham reopened this Nov 23, 2015

Ortham closed this as completed Nov 25, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use not equal comparsion for timestamps #25

Use not equal comparsion for timestamps #25

Ortham commented Nov 23, 2015

Ortham commented Nov 23, 2015

Use not equal comparsion for timestamps #25

Use not equal comparsion for timestamps #25

Comments

Ortham commented Nov 23, 2015

Ortham commented Nov 23, 2015