Rebased4 fix replay #214
Closed
Rebased4 fix replay #214
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… items
Global items moved from middle of .cpp file to class header, gaining 'm'
for member prefix:
cTelnet Class {private members}:
char loadBuffer[] ==> mLoadBuffer[]
int loadedBytes ==> mLoadedBytes
QDataStream replayStream ==> mReplayStream
QFile replayFile ==> mReplayFile
Items renamed to include 'm' for "member" prefix:
bool loadingReplay ==> mIsReplaying
mudlet class:
Items renamed to include 'mp' for "member pointer" prefix:
QAction * actionSpeedDisplay ==> mpActionSpeedDisplay
QAction * actionReplayTime ==> mpActionReplayTime
QLabel * replaySpeedDisplay ==> mpReplaySpeedDisplay
QLabel * replayTime ==> mpReplayTimeDisplay
QTimer * replayTimer ==> mpReplayTimer
QToolBar * replayToolBar ==> mpReplayToolBar
QAction * actionReplaySpeedUp ==> mpActionReplaySpeedUp
QAction * actionReplaySpeedDown ==> mpActionReplaySpeedDown
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
The icons style/sizes were not being set and were not modified by changes made in preferences. In Qt5 a NULL QTime is invalid and now adding a time to it no longer makes it valid (this is different from Qt4 where adding a time DID make it valid). Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
For replays where input occurs less often than once a second the time
display was only updated when each chunk was read with the result that the
time display could be static for long periods. This commit increments the
display by one second every second divided by the speed factor. A previous
attempt to do this caused some "jumping/sticking" in the display because
the displayed time was advanced only once a scaled second, which meant the
displayed time did not seem to increase uniformly as it was reset to the
right value when each chunk is processed. By decreasing the amount the
displayed time was incremented to a scaled tenth of a second the
perturbations is reduced considerably.
As the used time format {"hh:mm:ss"} is the default for the
QTime::toString() method used to generate the text to display on the Replay
Time display widget we do not need to have a const QString constant to hold
the time display format to use as the record of the formatting we want.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
This commit uses negative values of the mudlet class member mReplaySpeed to indicate speeds that are the reciprocal of the integer number. It tweaks the display of the speed to show a fractional value rather than a decimal one in the limited cases (1/2, 1/4, 1/8) that are coded for. As the lowest supported speed is altered by this commit it also seemed reasonable to set a reasonable upper limit of 128 so that one second of replay corresponds to just over two minutes of real-time in the original recording. At the same time the infrastructure to support translation of the widgets that contain text is implemented for future internationalisation. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…ing) Now needed for some work being done on fixing some bugs in the replay system. This is not the exact same code that was previously removed as the change to using QSharedPointer<Host> to access the mHostPool members instead of the previously used direct Host * pointers. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Generate and provide messages about both replay starting success and
failure for both main Toolbar and lua script initiated replays from
the same place {QString cTelnet::loadReplay(QString & filename,
bool "called from Main Toolbar")} so that the latter invocation will not
produce messages on the main console for the profile. Only for the former
case can a message be produced when a replay ends. A later commit will
provide system events related to replays to user scripts to overcome this
issue.
Remove redundant TConsole::loadRawFile(std::string) as all it does is
convert the file name given in a call from TLuaInterpreter::loadReplay()
into a pathFile relative to the profile's log directory before calling
cTelnet::loadReplay() and that can (and now is) done directly by the
TLuaInterpreter method revised here.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Reformat error message to current proposed "standard" and make it capable of being internationalised. Now return a Boolean on success or failure to start or stop logging and an (internationaliseable) message indicating what happened, if anything and including the log file-name if applicable. In light of the messages now available to the lua command the code has been revised so it will no longer cause a logging starting/stopping system message to appear on the main console - though using the button on the main console will continue to do so.) The existing log-to-file button used an icon that appears to have been an old Oxygen "folder-downloads.png" icon image and the only way to tell if it was active was a system dependent colouration that "on" widgets adopt. I have taken the current 48x48 Oxygen "text-x-log.png" and superimposed smaller (32x32) "media-record.png" and "media-stop.png" to create two similar in style but visibly different "log-record" and "log-stop" icons, they, like any other icons I have produced from the Oxygen icon set, are released under a GPLv2+ licence for use in Mudlet and anywhere else that is permitted. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Found that during testing with multiple profiles closing the profile that
was running the replay caused a crash - in theory it could also happen when
using a single profile if the timer that cause the next replay chunk to be
loaded occurs during close down. Cleanest fix is to provide a
cTelnet::abortReplay() member that duplicates the actions that would happen
at the end of the replay normally. Modifies the mudlet::replayOver(...)
method so that the affected profile gets a "myReplayAborted" rather than
"myReplayOver" "sysReplayEvent" event which is not likely to be much use
at profile close-down but could be of use if the toolbar or lua interpreter
gains a means to stop a replay {change the "disabled" during "replay"
toolbar button to an enabled "stop" one, or when an abortReplay() lua
command is added} in a following commit.
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…by new Also by using QPointer for the resources allocated with "new" we can avoid problems with tryinn to "delete" them if they no longer exist. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Until now the Mudlet replay files had the generic ".dat" file extension and
there was no protection against the user loading a file that is NOT a
replay file. Given that the file includes embedded values to control the
amount to read from the file using a time sent to a QTimer in another
embedded value this seems unwise. This commit adds some data to the start
of a replay file that serves to unique identify it (and adds a version code
so that if the format is changed in the future old formats can still be
identified correctly!) It also changes the extension to ".mreplay" to
stand for Mudlet replay, moving away from a three letter string as an
extension as that restriction went away in operating systems probably way
older than Mudlet ever supported. Old format replays will still be
acceptable, but the user will have to choose to use those by changing the
file type filter from the file requester dialog that is called when a
replay is requested from the main toolbar button.
The use of "dd-MM-yyyy#hh-mm-ss" as the time format to name the replay file
may look pretty but is unhelpful in sorting as an alphabetical sort of the
directory contents will group by day of month, month and then year etc.
This is OK provided the date-time meta-data is preserved but if such files
are archived or copied that date-time information can be lost and then
picking out files by age can become very difficult if the listing takes
more than one screen-full. Putting the most significant date-time item
first, as is common in some world regions, makes an alphanumeric sort
produce the same results as a date-time meta-data sort and preserves the
sort order if the latter data is corrupted.
In addition, by structuring some elements in the header in a particular
layout it makes it possible to determine some characteristics about the
file from the command line - this is particularly relevant on *nix
platforms which offer the file(1) command using data parsing routines
configured by magic(5).
The differences now mean instead of receiving the lowest common denominator
of file type information, the generic "data" type:
'$> file ./WoTMUD_Jomin/log/23-02-2014#12-31014.dat
./WoTMUD_Jomin/log/23-02-2014#12-31014.dat: data'
one instead can get:
'$> file ./WoTMUD_Jomin/log/2014-09-12#17-28-11.mreplay
./WoTMUD_Jomin/log/2014-09-12#17-28-11.mreplay: Mudlet replay file, version 2, started: Fri Sep 12 17:28.11 2014(UTC), 15.106(s) long, profile: "WoTMUD_Jomin"'
As it is possible to initiate a replay from with a user Lua script it does
not seem unreasonable to also provide commands to:
* abort a replay
* obtain various bits of status information about such a replay
* control the playback speed in the same way as the toolbar does.
Some existing class members/methods have been modified for clarity or other
reasons:
* TConsole::mReplayFile ==> TConsole::mRecordFile
* TConsole::mReplayStream ==> TConsole::mRecordStream
above two are associated with making a recording not with its playback
* TConsole::mRecordReplay ==> TConsole::mIsRecording
to match corresponding cTelnet::mIsReplaying.
* cTelnet::readPipe() ==> cTelnet::slot_readPipe()
to emphasise it is a slot method.
* QTime cTelnet::timeOffset ==> QDateTime cTelnet::mRecordLastDateTimeOffset
instead of measuring the elapsed time directly we now save the point in
time when the last chunk of data item was recorded, this allows for the
unlikely but possible case of records of longer than a day or for when
DST changes - both of which a QTime can not handle.
* mudlet::mpReplayTimer ==> mpReplayDisplayTimer
to differentiate it from the new mpReplayChunkTimer, the first is
associated with updating the time display periodically every
(second/speed) and the second with loading the next chunk after the
right time period.
Both are included in the class header so they can be accessed /
controlled from outside of the class which is not possible if they are
declared as class globals as the first had been in the past.
* cTelnet::abortReplay() ==> cTelnet::abortReplaying()
to match corresponding TConsole::abortRecording().
* cTelnet::mLoadingReplay ==> cTelnet::mIsReplaying
to match corresponding TConsole::mIsRecording.
Also:
mLogFileName, local to TConsole::slot_toggleReplayRecording() MASKS a
variable with the same name declared in the class header, it has been
renamed to recordingPathFile. Another local variable of the same name in
TConsole::slot_toggleLogging() had the same effect - the local definition
there has now been removed so that that method will use the original member
of the class instance after all.
In TConsole.h the member mpCommandLine has been taken out of the grouping
of public methods and re-positioned further down with the other public
members. Similarly the cTelnet::postMessage( QString ) method has been
moved out from the public members and up to the bottom of the other public
methods.
When making a recording Mudlet now should detect a failure to write out a
chunk of replay data completely and will abort, try to clean up the end of
the recording if possible, and report an error in the main profile console.
During the loading/playing of replays the data was being loaded into a
static fixed size char array { cTelnet::mLoadBuffer } of 100 Kilo-Bytes!
However as we know the size of each chunk before it is read from the file
it can easily be changed to a QByteArray that can be enlarged as required
if it is not big enough to hold a particular chunk of replay data. In
a quick test the array only grow to a few thousand bytes and will only
slightly slow things down for each chunk read for which the buffer is
undersized until the maximum size needed is reached.
Although not marked with any licence terms I hereby state that the "magic"
file entry added to the INSTALL file is, for the avoidance of any confusion
to be considered as licensed as "GPLv2+" - if that is not suitable then
please contact me for alternative options:
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…fine Not the most elegant way of controlling such debug output, only relevant to ctelnet.cpp and only changeable at compilation time. On the other hand it is better than what was used previously - well, a little bit...! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Closed
|
A bug in Travis is sending the OSX job to a Linux worker, so please check the build logs before relying on Travis' status... |
|
Stephen - what I meant was to return a tuple of the message + code, you would still need to make use of isStatusToBeReportedInConsole to choose which style of message should be given. It's just that if isStatusToBeReportedInConsole is passed in as true, when the function would be discarding the code. |
…y(...) After both subtle and then some more direct hints from another developer I (finally!) understood that it was better to past an integer code separately back from this method in a QPair<T1,T2> form rather than appending it to the front of the other return value of a QString. Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Ah, I eventually got what you were saying and have done that in this latest commit. Luckily it can go on the end of this series of commits and I will try and refrain from creating unnecessary PRs - I just don't like to mess up my own repo but if I'm a bit more careful not to push out to that until I'm more certain that will help everyone. 🙏 |
|
+1 I'm happy with this user-wise :) |
|
OK to merge? |
|
Both me and ahmed I'm fairly sure are drowning in rl work at the moment...So I can't really review a 1200 line merge at the moment. Hate to be a bottleneck, but I can't offer any insights into it or evaluate at the moment. |
|
Ping |
|
Well, yes - Pong...! On 07/12/14 21:55, Vadim Peretokin wrote:
|
| fileName = QDir::fromNativeSeparators( QString::fromUtf8( lua_tostring( L, 1 ) ) ); | ||
|
|
||
| Host * pHost = TLuaInterpreter::luaInterpreterMap[L]; | ||
| #if defined( TILDE_EXPANSION ) |
There was a problem hiding this comment.
Hm why a define? Seems like a good feature to have on all platforms.
There was a problem hiding this comment.
From what I recall it is described as a bashism - i.e. the use '~' on its own to refer to the user's home directory {not with standing the expansion to ~otherusername for other users' home directory} and is not something that users of some other OSs coming from a place called Redmond in the USofA are going to necessarily be familiar with...
There was a problem hiding this comment.
Yes, but we might as well not make it harder for scripts and force them to use ~ or something else - having ~ work everywhere will make scripts not require if/else conditions.
There was a problem hiding this comment.
I also use ~ in a lot of Mudlet forum posts, and explain that ~ is the home directory - I think people will pick knowledge up on that here and there anyway.
|
+1 looking good, including the Lua API |
| @@ -166,3 +166,21 @@ Host * HostManager::getFirstHost() | |||
| QMutexLocker locker(& mPoolLock); | |||
| return mHostPool.begin().value().data(); | |||
| } | |||
|
|
|||
| Host * HostManager::getNextHost( QString lastHost ) | |||
There was a problem hiding this comment.
What is the point of this function? The hosts are in a QMap, which is alphabetical so any specific ordering is lost.
|
This is a big change to look over the and the carry-over isn't marginal. Can you make a branch with just the changes in question? |
|
Ah, the debug stuff is actually relevant. Can you just comment on the getNextHost function? |
|
Away from the codebase ATM but IIRC I had to resurrect the getNextHost The odd change in the header was to shift the mpCommand line member down On 13/01/15 20:25, Chris Mitchell wrote:
|
|
I'll try and get around to scraping this up, dusting it off and resolving the conflicts with the current code - is it desirable to plumb it in the release_30 branch as well? |
|
It wasn't before, as that would have delayed 3.0 by the means of more testing needed for all code changes introduced - but at this point, it doesn't matter as much. So I'd say go ahead. |
mehulmathur16
pushed a commit
to mehulmathur16/Mudlet
that referenced
this pull request
Feb 16, 2024
fixes Mudlet#213 Also make `sentry_options_get_http_proxy` const-correct, fixes Mudlet#214
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right, hopefully I've taken on-board all the points raised about #200 others have made with the exception of:
* For a method that returned messages about the action it did or the error that prevented it from doing so that required two different style of message depending on the caller, returning both types and allowing the caller to choose which to use was not the optimum solution because it also had to configure whether another method would send a further message or not when that had finished. In other circumstances it might have been worth considering but not this time!