I’ve improved yesterday’s DimmedScreen hack-a-gadget. Following Jeff’s advice, I’ve optimised it to the point where it’s fast enough to be a useful class. It does have one problem that can only be overcome by clever GUI design, though - if gadgets beneath an instance of the DimmedScreen are animated, they will appear to freeze. This happens because the DimmedScreen is not animated, so doesn’t redraw every frame. You could make it redraw by registering it for VBLs, but then you’d be wasting all of your CPU time refreshing the entire framebuffer every frame.

Next up, I’ve created a “Requester” class. This is a compound gadget that inherits from the AmigaWindow and contains a listbox, an “OK” button and a “Cancel” button, and allows the user to choose from a set of options. When the user selects an option, either by double-clicking it or by clicking the “OK” button, the requester fires a “value changed” event and then closes itself. Clicking “cancel” just closes the window. An event handler that listens for the “value changed” event can, therefore, quite easily loop through the list of options and identify the selected ones.

Related to this, I’ve created a file requester using the same pattern. If you enable libfat support in your makefile and initialise it somewhere, you can drop the FileRequester files into your project (they’re in the “bonus” folder by default) and get an instant file requester. I’m surprised that it is fast, despite the very clunky string handling that it does presently to sort the file list (totally unoptimised sorted insert) and parse pathnames (even worse mix of a multitude of calls to strcmp(), strcat() and strlen()).

The file requester was one of my holy grails for this project, so I’m pleased that it’s working so well.

Comments

ant on 2008-05-02 at 21:20 said:

Goes back to the backwards compatible thing - that’s not a consideration at the moment. Liable to make things harder for people, definitely, but I’m hoping that a complete disregard for compatibility will produce a cleaner API at the end.

Copy constructor’s an interesting point. I”l have to take a look into how they work…

Nothing tragic, you just added a mandatory attribute to the Screen() constructor. It might have been sensible to add a default value for flags (ie, 0) which would have reduced problems on other coders. Just good practice, in my opinion (for what its worth)

Also, I note that the compiler thinks there is a copy constructor which triggered a bunch of extra paranoia in my head. It would probably make sense to prevent the compiler from synthesising those things in pretty much every class - I doubt that Woopsi has any classes that would do copy constructors correctly, but C++ beginners can invoke them without knowing if they are careful.

ant on 2008-05-03 at 08:24 said:

I’ve been through the code and replaced all instances of “char” with “const char”. I’ve also tidied up a lot of places where the code should have been making copies of the strings but was just assigning pointers instead.

Let me know how you get on with the latest version!

Jeff on 2008-05-03 at 08:34 said:

I just spent a chunk of the day wrestling with devkitARM r23 and have something working. I ran Woopsi through it, and it barfed:

The good news is that those all all just places where you have a ‘char *’ argument and are passing in a “literal string”. They should be changed to ‘const char *’

Jeff on 2008-05-03 at 08:40 said:

Copy Constructors.

In short, if C++ ever thinks you are wanting a ‘duplicate’ of an object (for example, if you called a routine that expected you to pass in a Gadget as opposed to a Gadget*) the compiler will silently create a new gadget and call the “copy constructor” which is the constructor of the form Gadget::Gadget(const Gadget &)

The big problem is that if you don’t provide a method of that form, the compiler will synthesise one by bit-copying. ie, it allocates a block of memory the same size and uses memmove(). Clearly that screws up any member variables that are supposed to be private.

The quick fix is to declare the copy constructor private - that stops anyone else from accidentally calling it. Don’t make it virtual, and don’t provide any content and you should catch any cases where it gets accidentally called from within the Gadget class itself without getting link errors.

I’ve been bitten by this where I passed an object to a 3rd party library (which was badly coded) and my object’s synthesised copy constructor was called from inside that library. I had to add an explicit copy constructor to avoid a bug in someone else…

ant on 2008-05-03 at 09:04 said:

I’m copying my development VM at the moment so that I can upgrade devkitPro in the copy and still keep r20. I should be able to sort the problems out more easily that way and save you some headaches!

ant on 2008-05-03 at 09:09 said:

Yikes, the PALib stuff is going to make a lot of work for someone…

Jeff on 2008-05-03 at 09:36 said:

I had to fix three source files to include - presumably the new headers don’t include them for you. You need it for access to abs(), rand(), atoi()

M source/pacghost.cpp
M source/calculator.cpp
M source/pacghosts.cpp

Jeff on 2008-05-03 at 09:38 said:

I think I spotted one of your ‘string duplications’ ;-)

/projects/Woopsi/woopsi/screen.cpp: In member function ‘void Screen::setTitle(const char*)’:
/projects/Woopsi/woopsi/screen.cpp:414: error: ‘strlen’ was not declared in this scope
/projects/Woopsi/woopsi/screen.cpp:417: error: ‘strcpy’ was not declared in this scope

Jeff on 2008-05-03 at 09:41 said:

The PA people won’t be happy either, I suspect. They have things like this:

ant on 2008-05-03 at 09:41 said:

Ah, I’ve upgraded to r22 then. I’ve fixed the problems that show up in this version, which seem to be the same ones that you picked up on. The only problem I’ve got is that I can’t build with this version - I get an error that says “Cannot open file ‘c:/devkitPro/libnds/default.arm7’“. It’s true that this file doesn’t exist, but I can’t see what is trying to include it.

Did I mention that I hate mucking about with makefiles? :(

EDIT: Ah, no - it is r23, at least according to the installed.ini file.

EDIT EDIT: On the assumption that I’m not using anything but the most basic ARM7 functionality, I copied libnds/basic.arm7 to libnds/default.arm7, which is probably a really bad idea, but it works fine. Hurrah!

Jeff on 2008-05-03 at 10:30 said:

Note, R23 isn’t generally available, and they don’t have an OSX version of R22 ready either. I’m surfing the bleeding edge, though I haven’t hit any problems yet…

The PA types can probably just disable the warning with -wsomethingorother

But it definitely looks like gcc is working further towards ‘const-correctness’ which is a good thing in the end.

ant on 2008-05-03 at 11:13 said:

Oh, I see - it’s libnds that’s behind. Yeah, that does make things rather confusing. I suppose the argument for having non-synchronous updates is that libnds sits on top of devkitPro along with a lot of other libraries, and the dev environment shouldn’t wait around for layers above it to update before a new version can be released. One lagging platform shouldn’t be able to hold up the release for all of the others.

ant on 2008-05-03 at 11:18 said:

Thanks for the directory stuff - I’ll have a look at it when RapidShare decides it wants to let me get at the files. The captcha they’ve started using is ridiculous and I managed to get it wrong twice. Bah.

Seriously, do you want some FTP space on this server? It’d make sharing files easier.

Jeff on 2008-05-03 at 12:02 said:

ndstool uses that file by default - that was one of the changes they made

fundamentally, despite announcements to the contrary, devkitpro and libnds do need to upgrade in lockstep