2008-05-02

File Requesters and More

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.

Woopsi File Requester

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…

Jeff on 2008-05-02 at 22:13 said:

Hmmm, at version 630, you broke my hpcalc sources.

/projects/ds-hpcalc/common/hpcalc.cpp: In member function ‘virtual void HPCalculator::startup()’: /projects/ds-hpcalc/common/hpcalc.cpp:82: error: no matching function for call to ‘Screen::Screen(const char [8])’ /Developer/NDS/libwoopsi/include/screen.h:26: note: candidates are: Screen::Screen(char, u32, FontBase) /Developer/NDS/libwoopsi/include/screen.h:18: note: Screen::Screen(const Screen&) /projects/ds-hpcalc/common/hpcalc.cpp:86: error: no matching function for call to ‘Screen::Screen(const char [9])’ /Developer/NDS/libwoopsi/include/screen.h:26: note: candidates are: Screen::Screen(char, u32, FontBase) /Developer/NDS/libwoopsi/include/screen.h:18: note: Screen::Screen(const Screen&)

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:

/projects/Woopsi/woopsi/alert.cpp: In constructor ‘Alert::Alert(s16, s16, u16, u16, char, char, FontBase)’: /projects/Woopsi/woopsi/alert.cpp:9: warning: deprecated conversion from string constant to ‘char

/projects/Woopsi/woopsi/cyclebutton.cpp: In constructor ‘CycleButton::CycleButton(s16, s16, u16, u16, FontBase)’: /projects/Woopsi/woopsi/cyclebutton.cpp:4: warning: deprecated conversion from string constant to ‘char

/projects/Woopsi/woopsi/debug.cpp: In member function ‘void Debug::output(char)’: /projects/Woopsi/woopsi/debug.cpp:42: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:45: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp: In member function ‘void Debug::createGUI()’: /projects/Woopsi/woopsi/debug.cpp:77: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:84: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:99: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:104: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:105: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:106: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:107: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/debug.cpp:109: warning: deprecated conversion from string constant to ‘char*’

/projects/Woopsi/woopsi/multilinetextbox.cpp: In constructor ‘MultiLineTextBox::MultiLineTextBox(s16, s16, u16, u16, char, u32, s16, FontBase)’: /projects/Woopsi/woopsi/multilinetextbox.cpp:19: warning: deprecated conversion from string constant to ‘char*’

/projects/Woopsi/woopsi/requester.cpp: In constructor ‘Requester::Requester(s16, s16, u16, u16, char, FontBase)’: /projects/Woopsi/woopsi/requester.cpp:35: warning: deprecated conversion from string constant to ‘char‘ /projects/Woopsi/woopsi/requester.cpp:44: warning: deprecated conversion from string constant to ‘char

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:

    if (PA_CompareText(PA_FSFile[FSImage].Ext, "bmp")){

which result in this:

Albatross:Woopsi jeff$ make -f makefile.app Compiling demo.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /Developer/NDS//libwoopsi/include/woopsiheaders.h:33, from /projects/Woopsi/source/demo.cpp:3: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling main.cpp Compiling pacghost.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pacghost.h:4, from /projects/Woopsi/source/pacghost.cpp:1: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling pacghosts.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pacghosts.h:4, from /projects/Woopsi/source/pacghosts.cpp:1: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling pacman.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pacsprite.h:4, from /projects/Woopsi/source/pacman.cpp:7: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling pacmap.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pacmap.h:4, from /projects/Woopsi/source/pacmap.cpp:1: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling pacplayer.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pacplayer.h:4, from /projects/Woopsi/source/pacplayer.cpp:1: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling pacsprite.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pacsprite.h:4, from /projects/Woopsi/source/pacsprite.cpp:1: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char‘ Compiling pong.cpp In file included from /Developer/NDS//PAlib/include/nds/PA9.h:39, from /Developer/NDS//libwoopsi/include/woopsifuncs.h:140, from /projects/Woopsi/source/pong.cpp:5: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadFSImage(u8, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:630: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:633: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:636: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h: In function ‘void PA_LoadGBFSImageToBuffer(void, s16, s16)’: /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:658: warning: deprecated conversion from string constant to ‘char‘ /Developer/NDS//PAlib/include/nds/arm9/PA_Draw.h:664: warning: deprecated conversion from string constant to ‘char

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!

EDIT EDIT EDIT: r23 came out on the 29th of April: http://www.devkitpro.org/devkitarm/devkitarm-release-23-now-available/

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

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

since you’re racing ahead with your file selector, take a look at

http://rapidshare.com/files/112218505/nds_fat.cpp.html

it has a few tweaks in it that make the directory functions work in the SDL build (ie, when libfat isn’t available) - feel free to borrow/steal

Jeff on 2008-05-07 at 12:04 said:

It looks like you missed one when fixing Alert.cpp

/projects/Woopsi/woopsi/alert.cpp: In constructor ‘Alert::Alert(s16, s16, u16, u16, const char, const char, FontBase)’: /projects/Woopsi/woopsi/alert.cpp:9: warning: deprecated conversion from string constant to ‘char

8   // Define OK button text
9   char* buttonText = "OK";

needs to be ‘const char *’

Sorry about the rapidshare captcha - I don’t see it. Its makes it too damned easy to shift large files between here and my brother in Canada so I’ve paid the premium…

As to ftp space, the next time I have stuff to update, I’ll give you a yell

Jeff on 2008-05-07 at 12:23 said:

To silence the warnings about const-strings in the PA headers, you should add -Wno-write-strings to the CFLAGS define in your makefile.

Its not a good long-term fix, but it’ll make it easier to see real errors…

ant on 2008-05-12 at 14:30 said:

Fixed the Alert const prob.