Enhance: overhaul stopwatches redux#3224
Conversation
This commit is a rework of the original PR Mudlet#2516 squash-and-merged with the bugfix that I proposed as PR Mudlet#3162 but which became difficult to apply when the original PR was eliminated from the development branch because it has been reverted from the 4.2.0 release and then that had been merged into the development branch! This PR allows stopwatches to be stopped and read at any time afterwards, for any number of times. It also: * allows stopwatches to be destroyed - so ID numbers WILL get reused - with a Lua (bool) destroyStopWatch((int) id) function. * allows them to be marked as persistent so that they are saved with the profile and reloaded again - if they were running then they will continue to increment when the profile is not loaded so they can be used to real time events outside of the profile/session; this uses a new Lua function (bool) setStopWatchPersistence((int) id, (bool) setPersistent). * allows them to be adjusted (even so they become negative) so can be used to count down as well as count up time - whether the stopwatch concerned is running or not. * have error messages that conform to our current style * have more run-time handling - most actions that produce no effect will advise that this is the case - e.g. stopping a stopwatch that was NOT running... * can handle periods of time longer than a day - the previous implementation wrapped around after 24 hours. * should not affected by DST changes - OS permitting. * the Lua API also gains a getStopWatches() function that returns a table with the id numbers as keys and values as tables of: * (bool) isRunning * (bool) isPersistent * (string) name * (table) elapsedTime - containing broken down time: * (bool) negative * (int) days * (int) hours (0 to 23) * (int) minutes (0 to 59) * (int) seconds (0 to 59) * (int) milliSeconds (0 to 999) * (float) decimalSeconds floating point value of whole of the time in seconds (can be negative!) * allows stop watches to be named - which is useful as then they can be identified in scripts; this means that all the stop watch functions can now take a name string as well as a numeric argument. Using an empty string will access the first (lowest id) stopwatch that does not have a name and the createStopWatch() function will accept an optional string argument as a name. For simplicity each name must be unique and this is enforced for that function and the added setStopWatchName(id or name, newName) function. The latter will also accept an empty string as either the first or second argument; in the first case it will assign the name to the first unnamed stopwatch and the second will clear the name of the specified one. * added getStopWatchBrokenDownTime(...) which returns the same broken down elements in a table for a single specified timer (day count; hours; minutes; seconds; milliseconds and whether the time is positive or negative {when preset with a negative adjustment and used as a count down})... During debugging I found out that the process of loading an existing profile that contained createStopWatch() calls was creating them during the testing/loading phase so I had to add extra code to prevent that function from taking effect whilst (bool) ~~`Host::mIsProfileLoadingSequence`~~ *revised to use a different, new, flag: `Host::mBlockStopWatchCreation` which is cleared earlier in the loading sequence* is true. Then, when testing the resetProfile() function I found that the same thing was happening AND that the non-persistent stopwatches needed to be removed as well - which is now solved by also preventing stopwatch creation whilst (bool) Host::mResetProfile is set and by running a new method (void) Host::removeAllNonPersistentStopWatches()! This thus allows stopwatches to be created during the profile startup sequence when lua scripts are run on loading (but after a prior compilation to test for script validity has been done). The code in (void) stopWatch::Host::adjustMilliSeconds(const qint64) originally used (QDateTime) QDateTime::addMSecs(qint64) incorrectly in that I had thought it adjusted the QDateTime it was called upon whereas it returns a reference to the adjusted value - so needed to be invoked in a different manner which was already being done in the original PR. QString stopWatch::getElapsedDayTimeString() const made use of int for some intermediate local variables but on Windows platforms the int type may be 32-bit long and thus not big enough to contain 64-bit values - and some other locals can be much shorter because of the limits that the code places on their values but will give compiler warnings without static_cast <T>. Use std::chrono_literals to specify some needed time intervals rather than large long integer literal constants. Refactor a block of code used to generate the broken down time as a Lua table so that two instances are handled by a single helper function. Additional code (the bug fix) has been included to provide backwards-compatibility for Lua startStopWatch(id) function which recreates prior start stop-watch behaviour when it is created: The prior form of startStopWatch(...) would reset and restart the indicated stopwatch each time that it was called. This is not really compatible with the revised functionality which allows the recorded time to be adjusted even before the stopwatch is first used. To allow existing scripts to continue to experience the same API this commit adds an optional second boolean argument to the startStopWatch(...) call ONLY WHEN the first argument is an id number and NOT a string name. If the second argument is omitted (as it will be when using older scripts) or is true then each time the function is used the stopwatch will be reset and restarted. Just in case the same behaviour ***is*** wanted with stop watch created with a **name** then a second boolean `true` will start the stopwatch from zero. This PR replaces (and thus will) close Mudlet#3162 . Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
…verhaulStopwatches
|
I don't have time to extensively test this, but a little thing, codefactor is rightly pointing out duplication in Mudlet/src/TLuaInterpreter.cpp Lines 2134 to 2152 in 6ab9db1 Mudlet/src/TLuaInterpreter.cpp Lines 2179 to 2197 in 6ab9db1 Mudlet/src/TLuaInterpreter.cpp Lines 2218 to 2236 in 6ab9db1 Mudlet/src/TLuaInterpreter.cpp Lines 2340 to 2358 in 6ab9db1 |
Created a single function that is used to do the same thing in four separate stop-watch functions. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
@PhoenixCodes checked this out and says the ire mudlet mapper works fine! |
| class dlgNotepad; | ||
| class TMap; | ||
|
|
||
| class stopWatch { |
There was a problem hiding this comment.
Shouldn't this be StopWatch since all other class names are capitalised?
There was a problem hiding this comment.
Um, look at line 58 - that does not have an initial capital either! 😜
Unless we make it a style requirement that all Mudlet created classes have a particular naming style I am happy to leave it unchanged so as to get this epic topic (I was going to say PR but it of course is actually spread over more than one PR attempt) merged in...
There was a problem hiding this comment.
... we can, of course, tweak such naming conventions (and that of the files that contain them) in a separate PR if that is felt to be warranted (I guess the GLWidget is the opposite case).
Redo_Enhance: overhaul stopwatches This PR is a rework of the original PR Mudlet#2516 squash-and-merged with the bugfix that I proposed as PR Mudlet#3162 but which became difficult to apply when the original PR was eliminated from the development branch because it has been reverted from the 4.2.0 release and then that had been merged into the development branch! This PR allows stopwatches to be stopped and read at any time afterwards, for any number of times. It also: * allows stopwatches to be destroyed - so ID numbers WILL get reused - with a Lua (bool) destroyStopWatch((int) id) function. * allows them to be marked as persistent so that they are saved with the profile and reloaded again - if they were running then they will continue to increment when the profile is not loaded so they can be used to real time events outside of the profile/session; this uses a new Lua function (bool) setStopWatchPersistence((int) id, (bool) setPersistent). * allows them to be adjusted (even so they become negative) so can be used to count down as well as count up time - whether the stopwatch concerned is running or not. * have error messages that conform to our current style * have more run-time handling - most actions that produce no effect will advise that this is the case - e.g. stopping a stopwatch that was NOT running... * can handle periods of time longer than a day - the previous implementation wrapped around after 24 hours. * should not affected by DST changes - OS permitting. * the Lua API also gains a getStopWatches() function that returns a table with the id numbers as keys and values as tables of: * (bool) isRunning * (bool) isPersistent * (string) name * (table) elapsedTime - containing broken down time: * (bool) negative * (int) days * (int) hours (0 to 23) * (int) minutes (0 to 59) * (int) seconds (0 to 59) * (int) milliSeconds (0 to 999) * (float) decimalSeconds floating point value of whole of the time in seconds (can be negative!) * allows stop watches to be named - which is useful as then they can be identified in scripts; this means that all the stop watch functions can now take a name string as well as a numeric argument. Using an empty string will access the first (lowest id) stopwatch that does not have a name and the createStopWatch() function will accept an optional string argument as a name. For simplicity each name must be unique and this is enforced for that function and the added setStopWatchName(id or name, newName) function. The latter will also accept an empty string as either the first or second argument; in the first case it will assign the name to the first unnamed stopwatch and the second will clear the name of the specified one. * added getStopWatchBrokenDownTime(...) which returns the same broken down elements in a table for a single specified timer (day count; hours; minutes; seconds; milliseconds and whether the time is positive or negative {when preset with a negative adjustment and used as a count down})... During debugging I found out that the process of loading an existing profile that contained createStopWatch() calls was creating them during the testing/loading phase so I had to add extra code to prevent that function from taking effect whilst (bool) ~~`Host::mIsProfileLoadingSequence`~~ *revised to use a different, new, flag: `Host::mBlockStopWatchCreation` which is cleared earlier in the loading sequence* is true. Then, when testing the resetProfile() function I found that the same thing was happening AND that the non-persistent stopwatches needed to be removed as well - which is now solved by also preventing stopwatch creation whilst (bool) Host::mResetProfile is set and by running a new method (void) Host::removeAllNonPersistentStopWatches()! This thus allows stopwatches to be created during the profile startup sequence when lua scripts are run on loading (but after a prior compilation to test for script validity has been done). The code in (void) stopWatch::Host::adjustMilliSeconds(const qint64) originally used (QDateTime) QDateTime::addMSecs(qint64) incorrectly in that I had thought it adjusted the QDateTime it was called upon whereas it returns a reference to the adjusted value - so needed to be invoked in a different manner which was already being done in the original PR. QString stopWatch::getElapsedDayTimeString() const made use of int for some intermediate local variables but on Windows platforms the int type may be 32-bit long and thus not big enough to contain 64-bit values - and some other locals can be much shorter because of the limits that the code places on their values but will give compiler warnings without static_cast <T>. Use std::chrono_literals to specify some needed time intervals rather than large long integer literal constants. Refactor a block of code used to generate the broken down time as a Lua table so that two instances are handled by a single helper function. Additional code (the bug fix) has been included to provide backwards-compatibility for Lua startStopWatch(id) function which recreates prior start stop-watch behaviour when it is created: The prior form of startStopWatch(...) would reset and restart the indicated stopwatch each time that it was called. This is not really compatible with the revised functionality which allows the recorded time to be adjusted even before the stopwatch is first used. To allow existing scripts to continue to experience the same API this commit adds an optional second boolean argument to the startStopWatch(...) call ONLY WHEN the first argument is an id number and NOT a string name. If the second argument is omitted (as it will be when using older scripts) or is true then each time the function is used the stopwatch will be reset and restarted. Just in case the same behaviour ***is*** wanted with stop watch created with a **name** then a second boolean `true` will start the stopwatch from zero. This PR replaces (and thus will) close Mudlet#3162 . A separate commit was appended and squashed in to reduce a code duplication pointed out by CodeFactor: Created a single function that is used to do the same thing in four separate stop-watch functions. Signed-off by: Stephen Lyons <slysven@virginmedia.com
This commit is a rework of the original PR #2516 squash-and-merged with the bugfix that I proposed as PR #3162 but which became difficult to apply when the original PR was eliminated from the development branch because it has been reverted from the 4.2.0 release and then that had been merged into the development branch!
This PR allows stopwatches to be stopped and read at any time afterwards, for any number of times. It also:
(bool) destroyStopWatch((int) id)function.(bool) setStopWatchPersistence((int) id, (bool) setPersistent).getStopWatches()function that returns a table with the id numbers as keys and values as tables of:(bool) isRunning(bool) isPersistent(string) name(table) elapsedTime- containing broken down time:(bool) negative(int) days(int) hours (0 to 23)(int) minutes (0 to 59)(int) seconds (0 to 59)(int) milliSeconds (0 to 999)(float) decimalSecondsfloating point value of whole of the time in seconds (can be negative!)createStopWatch()function will accept an optional string argument as a name. For simplicity each name must be unique and this is enforced for that function and the addedsetStopWatchName(id or name, newName)function. The latter will also accept an empty string as either the first or second argument; in the first case it will assign the name to the first unnamed stopwatch and the second will clear the name of the specified one.getStopWatchBrokenDownTime(...)which returns the same broken down elements in a table for a single specified timer (day count; hours; minutes; seconds; milliseconds and whether the time is positive or negative {when preset with a negative adjustment and used as a count down})...During debugging I found out that the process of loading an existing profile that contained
createStopWatch()calls was creating them during the testing/loading phase so I had to add extra code to prevent that function from taking effect whilst(bool)EDIT: revised to use a different, new, flag:Host::mIsProfileLoadingSequenceHost::mBlockStopWatchCreationwhich is cleared earlier in the loading sequence is true. Then, when testing theresetProfile()function I found that the same thing was happening AND that the non-persistent stopwatches needed to be removed as well - which is solved by also preventing stopwatch creation whilst(bool) Host::mResetProfileis set and by running a new method(void) Host::removeAllNonPersistentStopWatches()! This thus allows stopwatches to be created during the profile startup sequence when lua scripts are run on loading (but after a prior compilation to test for script validity has been done).(QString) stopWatch::getElapsedDayTimeString() constmade use ofintfor some intermediate local variables but on Windows platforms theinttype may be 32-bit long and thus not big enough to contain 64-bit values - and some other locals can be much shorter because of the limits that the code places on their values but will give compiler warnings withoutstatic_cast <T>.Use
std::chrono_literalsto specify some needed time intervals rather than large long integer literal constants.Refactor a block of code used to generate the broken down time as a Lua table so that two instances are handled by a single helper function.
Additional code (the bug fix) has been included to provide backwards-compatibility for Lua
startStopWatch(id)function which recreates prior start stop-watch behaviour when it is created:The prior form of
startStopWatch(...)would reset and restart the indicated stopwatch each time that it was called. This is not really compatible with the revised functionality which allows the recorded time to be adjusted even before the stopwatch is first used. To allow existing scripts tocontinue to experience the same API this commit adds an optional second boolean argument to the
startStopWatch(...)call ONLY WHEN the first argument is an id number and NOT a string name. If the second argument is omitted (as it will be when using older scripts) or is true then each time the function is used the stopwatch will be reset and restarted. Just in case the same behaviour is wanted with stop watch created with a name then a second booleantruewill start the stopwatch from zero.This PR replaces (and thus will) close #3162 .
Signed-off-by: Stephen Lyons slysven@virginmedia.com