Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

Code (wxCatapult) Log


Commit Date  
[ec38d2] by Patrick van Arkel Patrick van Arkel

this git is getting nitty gritty with Git (aka updated documentation to make Git a name)

2013-08-19 04:59:16 Tree
[2bccef] by wouter wouter

Simplify 'type ;' fix

The intention of the tclEscapeWord() functions is to escape _all_ characters
with special meaning in Tcl. So that just _any_ string can be passed as a
(single) parameter to a Tcl proc. Including strings that contain ';' characters
(the ';' character separates two Tcl commands (just like '\n')).

So the previous patch did fix the type command, but it did not fix all other
commands that used tclEscapeWord() in combination with ';'.

2013-08-18 09:05:50 Tree
[ea8d61] by joxy joxy

previous commit fixed

2013-08-18 04:28:23 Tree
[fdc8f8] by joxy joxy

A fix for Patrick's privately reported issue:

(13:07:39) Patrick: let catapult type
(13:07:43) Patrick: ;doesn't type
(13:07:50) Patrick: include the ; at the front
(13:08:00) egp_: that's probably TCL stuff...
(13:08:11) Patrick: yes
(13:09:55) egp_: this is a problem of OpenMSX, not of wxCatapult
(13:10:04) Patrick: yes
(13:10:21) egp_: try to enter the following string:

type ;doesn't type

at the OpenMSX console
(13:10:34) Patrick: I know
(13:10:39) egp_: it will respond with Syntax error...
(13:10:59) Patrick: type ";doesn't type"
(13:11:27) Patrick: that works
(13:11:52) egp_: and this works too:

type ";doesn't \"type\""

(13:12:02) Patrick: yes

2013-08-18 04:21:22 Tree
[ada103] by wouter wouter

Hack: don't delete m_connectThread

Vampier reported a crash on exit. This was in windows-specific code while
destroying the m_connectThread object. I recently added this because before it
was leaked. Though a leak is better than a crash, so I restored the old
behavior. It's not a big leak: only a few kB (or less?) each time openMSX is
stopped. Someone with a deeper understanding of wxWidgets could look into this,
but for me this is low priority.

2013-08-17 08:52:16 Tree
[bf4de8] by wouter wouter

Don't use exceptions to handle WriteMessage() errors

We briefly discussed this on IRC, and I agreed that using exceptions for this
seemed like a good idea. Though now that I've seen the actual implementation I
have to change my mind:

Exceptions become useful when the throw-site and the catch-site are far apart
in the code (lot's of function calls in between). Or when the caller has more
specific information it can add to the error. Or when the error has to be
handled in a different way depending on the context. In the current code none
of these reasons apply: WriteMessage() throws an exception. There are only two
direct callers and both immediately handle the exception in exactly the same
way.

The code becomes much simpler if instead of throwing an exception (and already
one level up handling it), we directly handle the error in WriteMessage().
(This also makes it easier to see that the actual printed message was quite
redundant.)

Also the exception class inheritance hierarchy was (IMHO) over-designed: there
was an abstract base class, one subclass that holds a string and another that
holds an error code. If there are lots of exceptions (stored in memory at the
same time) or if we try to catch very specific exceptions (bad idea in
general), then this design makes sense. Though in this case the only thing we
ever did with the errno-exception was turn it into a string. So we could as
well do that while constructing the exception, and then only have one exception
class (without inheritance) that stores this string.

So together with the previous patch this implements the error handling with
only 15 lines of extra code ;-) If in the future we refactor the code so that
WriteMessage() and the location where the error is handled are further apart,
then (part of) this patch may be reverted. But I still think the inheritance
thing was overkill. (E.g. we might want to kill the openMSX process and revert
the GUI to the 'stopped' state in case of an error .. although I think the case
were the openMSX process dies is already handled fine, so I prefer to first see
an example of another scenario where this error can occur before complicating
the code.)

2013-08-16 10:11:29 Tree
[3ca0ee] by wouter wouter

Various simplifications in WriteMessage error handling

There was a very complex routine to transform the error code 'errno' into a
wxString message. It used 'strerror_r()' and had logic to deal with dynamically
growing buffer sizes. It also dealt with the difference between the XSI and the
GNU variant of this function. But there is a much simpler function: strerror().
This function is not thread-safe, though as long as you only access it from the
main thread (as is the case for wxcatapult) then everything should be just
fine. (BTW: the buffer management in the old code was also rather bad. In
modern c++ code (especially c++11), you should rarely have to write 'new' and
'delete' explicitly, instead you should use 'managed' (memory) resources. In
this specific case 'vector<char>' could be used to manage the memory
allocation.)

Never use exception specifications (throw annotations) in c++ code. It was a
bad idea in the c++ standard. It's a deprecated feature in c++11. This (very
old) article explains why:
http://www.gotw.ca/publications/mill22.htm
Very short summary: c++ and Java exception specification syntactically look
similar, but it are very different things:
Java: they are statically checked by the compiler
C++: they are dynamically checked at run-time
When in Java you violate an exception specification, the program won't compile,
in c++ the code compiles (even without warnings) but at-run-time the program
gets terminated.

If you inherit from std::exception (or any of the more specific exception
types), you're expected to override the what() method. This returns a 'const
char*' with a description of the error that occurred. Though the standard
doesn't say anything about the encoding of this string. Also we want this
message as a wxString and it would be weird to convert the message back and
forth between 'const char*' and 'wxString'. So I just dropped the inheritance
from std::exception. Since we don't have a top-level catch block that attempts
to handle all possible exceptions, there was anyway no advantage in using
std::exception.

In c++ it's not needed to write empty constructors/destructors. It's also not
needed to explicitly call the default constructor of a base class. Especially
in very small classes (like the exception classes in this patch), the general
practice is to leave out these redundant constructs.

See also next patch for more on the exceptions...

2013-08-16 10:01:08 Tree
[1de588] by Manuel Bilderbeek Manuel Bilderbeek

Remove 2 one-liner methods and replace the single caller with their content.

2013-08-15 20:32:34 Tree
[91dfc9] by Manuel Bilderbeek Manuel Bilderbeek

To experiment how easy it is to add settings now after Wouter's refactorings, I added a Video Source and Disable Sprites setting in no time. Great job, Wouter

2013-08-15 19:51:41 Tree
[3715af] by joxy joxy

MSVC is not standards compliant, as people tell. A fix for it to not
issue warnings.

2013-08-15 15:56:46 Tree
[c30cb8] by joxy joxy

fixed MSVC compile error

2013-08-15 15:27:47 Tree
[318d0c] by joxy joxy

Small opimization

2013-08-15 14:02:36 Tree
[158450] by joxy joxy

A cleaner version of errno handling

2013-08-15 13:52:27 Tree
[bffebf] by joxy joxy

a fix: wxT instead of L"..."

2013-08-15 12:49:43 Tree
[3c99d4] by joxy joxy

handled errors in WriteMessage (non-Windows for now)

2013-08-15 12:46:09 Tree
[0e7505] by wouter wouter

Fixed 'Cart A' / 'Cart B' buttons

Was a regression I introduced in revision 8fe4576. Fixed now. At the same time
I made the disk/cart/cas button handling code more similar (and simpler).

2013-08-15 10:15:02 Tree
[83a6d6] by wouter wouter

Refactor media loop code

SessionPage.cpp had in some places a loop that went over all media (2 x disk,
2 x cart, cassette). To accomplish this it build a local array with pointers to
the media member variables and then looped with a custom (ugly) FOREACH macro.
The code becomes simpler if we just store the media variables directly in an
array.

2013-08-13 14:31:57 Tree
[399347] by wouter wouter

Factor out common code bewteen {disk,cart}{a,b}

The code that handled diska was copy/pasted to handle diskb. This patch tries
to have only one set of routines that are parameterized on the disk (a or b).
Similar for carta and cartb.

There still is code duplication between disks, carts and cassette. E.g. the
code to insert/remove a disk/cart/cassette is still very similar (but not
completely the same).

2013-08-13 14:10:44 Tree
[481055] by wouter wouter

Use unique_ptr to simplify memory managment

Also fix a few memory leaks.

2013-08-13 10:40:52 Tree
[b39b2c] by wouter wouter

Better encapsulate StatusPage

Before this patch StatusPage basically only exposed a public wxTextCtrl
variable. All code that wanted to print a status message had to manipulate this
widget directly. This patch makes that variable private and instead adds
public methods to print and clear. This also reduces some code duplication.

Before this patch internal error messages were shown in bold in the status
page. Since these are only internal error messages (should normally never
trigger) I didn't care about adding an option for this in the new access
methods (so now they're no longer printed bold, they still do have a different
color).

2013-08-13 10:31:15 Tree
[0ffcea] by wouter wouter

Cleanup openMSXController <-> wxCatapultFrame interaction

The class 'openMSXController' should concentrate on the interaction with the
openMSX process, the class 'wxCatapultFrame' should concentrate on the GUI
stuff. Though in the current code these two tasks are not well separated.

This patch adds a cleaner separation during start/stop. This allowed to replace
a bunch of public methods with only two new public methods: OpenMSXStarted()
and OpenMSXStopped(). The former public methods are now private. There was even
a public member variable, that's now also private.

2013-08-13 10:25:16 Tree
[42a0ae] by wouter wouter

Split StartOpenMSX() in two

This function took a boolean (2nd) parameter, depending on this value the
function did two very different things. Basically the whole function was a
if-then-else statement with one function in the then- and the other in the
else-block. There was only a single common statement in front of this if, but I
believe that statement only makes sense for one of the two blocks. (Ok, there
was also some boilerplate code around the if to handle the return value, but
actually only one of the two functions needed a return value).

So now there is a new function CheckVersion(). This will start openmsx with the
option '-v' (this will only print version information and exit). The function
then parses the output and checks for a minimum openMSX version. That's it. The
old function also executed some setup code (e.g. enable some GUI button), that
code is now moved. I think this was actually a bug when this function was
called via the 'Setting > Edit Configuration ...' menu.

The other part is still called StartOpenMSX(). This will actually launch
openMSX (and leave it running) and setup the communication channel(s) with the
openMSX process.

2013-08-13 09:22:22 Tree
[0b4ab7] by wouter wouter

Simplify GetOpenMSXVersionInfo()

The GetOpenMSXVersionInfo() method had a windows and non-windows specific
implementation. Though the windows version only used generic wxWidget functions,
so it can work as well on non-windows systems.

So this patch removes the non-windows specific (actually non-generic) code.
Because GetOpenMSXVersionInfo() is now so small I've inlined it in the (only)
caller.

An extra advantage of the generic version is that it doesn't leave a temporary
file '/tmp/catapult.tmp' behind (the code used this file but forgot to delete
it).

2013-08-13 09:22:15 Tree
[af63d9] by wouter wouter

Final simplifications to launch script infrastructure

The launch script code executed a hard coded method when the end of the script
was reached. But we can as well call this method from (the last command of) the
script.

This patch also restructures the code slightly: put functions in a more logical
order and rename them.

2013-08-12 15:20:20 Tree
[b488b2] by wouter wouter

Remove loop handling from launch script code

The previous patch already explained that '*' was still handled special in a
launch command. When this character was seen, it got replaced with each element
from a previously returned list. This patch completely removes that special
logic from the launch script code and instead implements it in specific
callback routines. Implementing it there was actually fairly easy now that we
have an improved WriteCommand() function with callbacks. And as a bonus it's
now trivial to re-introduce the 'parallelism' that was removed in the previous
patch.

This removed the last 'special interpretation' that the launch script code gave
to the commands. Now the launch script is just a collection of Tcl commands that
are send (unmodified) to openMSX. Each command has an associated callback that
is executed on a (successful) reply on the command. The next command is only
send after a reply of the previous command is received (successful or not).

2013-08-12 14:57:13 Tree
Older >