#8479 closed patch
New GP32 port
| Reported by: | SF/wonst719 | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | Port: GP32 |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
This is a restructured / remade version of current GP32 port in CVS. ScummVM now works on GP32, but it is unfinished and is WIP.
Some notes: - NP/BP/LP function and various macros that I added in portdefs.h eases my debugging process. - I made slight change to scumm.cpp. This allows GP32 to detect V5+ games properly though I don't know why this works. :)
Ticket imported from: #1341626. Ticket imported from: patches/584.
Attachments (3)
Change History (17)
by , 19 years ago
| Attachment: | scummvm-gp32.diff.gz added |
|---|
comment:1 by , 19 years ago
| Owner: | set to |
|---|
comment:2 by , 19 years ago
Within your port, you can do (almost) everything you like, so you don't have to "justify" using those :-). Some notes, however:
* You seem to add custom font data (in fontdata.c). Is that really necessary? Can't you just use one of the already built-in fonts?
* Why do you disable the mixer check in scumm.cpp ? At the very least, add a comment which explains the reasons.
* Your other change in scumm.cpp (I assume this is the one you are talking about in your submission), the one which moves "ScummGameSettings game = *g;" around, looks suspicious. If the two behave differently, something is quite wrong. Maybe a compiler bug?
comment:3 by , 19 years ago
* Custom font data is not really necessary. But I needed a smallar font (8x8) to display more debug lines at bottom of the screen.
* Mixer hack... Well, In fact, I forgot removing it from source code. :) I once revealed the hack, and then found the mixer works well without it. I will remove it soon.
* Change in scumm.cpp: I really don't know why it works. The two should behave the same. But the two definitely show different results. Without this change, game filename (g->name there) will be cut except its first letter. e.g) If selected game file name is "tentacle".000, the remaining will be "t".000. A compiler (arm-elf-gcc 4.0.1) bug is more likely.
comment:4 by , 19 years ago
* The console font is 5x7. That's not small enough? :-)
* Glad to hear about the mixer hack going away :-)
comment:5 by , 19 years ago
I uploaded a updated patch.
* Font: Unfortunately I can't use the font. Debug output starts BEFORE ScummVM is inited... * Removed mixer hack. :) * Optimized blitter a bit. * Mapped more buttons.
comment:6 by , 19 years ago
Ok. Changes to common ScummVM code are minimal and most are same as for other smaller platforms really nice.
Only inconvenience with the patch is that files from CVS directories were added too.
Questions/suggestions about backend itself. o Code formatting :( Your code is a big mix. I counted at least three different ways of putting opening square brackets ;) .See http://www.scummvm.org/documentation.php?view=conventions o Why clock speed is hardcoded? o Those hardcoded scummvm command line arguments. They could successfully be overridden in .ini file. Also -d9 usually produces _lots_ of output, so you either slow down whole process significantly or will fill in space on SMC in less than dozen minutes. o You don't really need to handle cursor target scale. That's intended for 640x480. o You have aspect ratio correction disabled. Probably you should do it only when debug level is set? o rename memcpy.S to memcpy.s o README :)))
Otherwise looks nice.
comment:7 by , 19 years ago
Won Star,
Firstly I was really pleased to see the version you released on your site and even more pleased to see it put up as a patch.
There is some much needed refactoring in this patch and it is a big improvement over the existing CVS code however there are a few things that you may want to consider putting on the TODO list. I dont mind taking a look at some of this once I get the GP2X port into a usable state if you want as I still have a soft spot for the port (and have some code outside CVS but the scale of the refactor makes most (but not all) of it irrelevant). Its nice to see minimal changes outside the backend and some of the more blatant hacks gone :).
It seems worth considering moving or rewriting the existing (admittedly hacky) pre-launcher that is in CVS into a backend constructor allowing you to setup any GP32 specific environment stuff before you go into the main engine and GUI. This should allow you to get selectable CPU speed support without all the timers going mad, enable/disable synth sounds etc. quite easily without the need to hack about with fake command line parameters and hard coding the executable.
Have you considered re-implementing the gammatab function that is in the CVS code (or something like it). This would IMHO be a worthwhile exercise as the quality and brightness of GP32s screens can vary wildly among the various models and can make ScummVM basically unusable on some devices without the gamma increase.
A similar comment comes from the removal of the virtual keyboard code (ok, keyboard is a maybe an overstatement) that allowed input (up/down to select char, left/right move about the text etc.) into text fields for things such as saves. Working on a more pragmatic approach looking at the Smallscreen backend ideas mused here http://sourceforge.net/tracker/index.php?func=detail&aid=1325302&group_id=37116&atid=418822 may be more suitable in the long run depending on the bloat added by inheriting such a potential backend considering how tight on RAM the port is.
I think you will need to gracefully handle 8.3 file names in the save engine. My hack to do this was never fit for committing to CVS ;) so you may consider something a little nicer (I setup a define of SHORT_SAVENAMES to wrap this so other platforms could use it, it is stil defined the makefile). Several of the games generate a savefile name over the 8.3 boundary and without a bit of work the GP32 lacks vFAT support so 8.3 is your lot and the save will fail.
Also, is the Fontdata.c the font from the GamePark SDK (not been able to check yet) as the fonts in the GamePark SDK whilst free are not strictly GPL if I remember correctly. Whilst thinking about the GPL, am I right in thinking the NetBSD derived memcopy.s you are using would require ScummVM to state This product includes software developed by the NetBSD Foundation, Inc. and its contributors. somewhere in the documentation? I seem to remember seeing people fall foul of that before not that it is a problem.
I am reviewing the version 2 DIFFs in more detail so I will add any more comments I have but it does look good.
comment:8 by , 19 years ago
Sorry, I'm late. :) I've fixed my code so code formatting follows ScummVM conventions. I'm currently working on pre-launcher and gammatab stuffs, because I felt some inconveniences without them. :) Hardcoded command line arguments can be removed. I'm also thinking about virtual keyboard like Symbian.
Notes: * fontdata.c is not one from GPSDK... but I don't think it's GPL, either. Well, I could make one and release it as GPL. But lack of time prevents me from working on this. :) * NetBSD license should be in README.gp32 / LICENSE.gp32 file...
Below's my current TODO list.
* Change the way how button is handled. Need "polling" events... * Reimplement pre-launcher so I can get rid of hardcoded cpuspeed and command-line argument things. * Implement gammatab * Figure out why playing sound makes crash occasionally. * Implement virtual keyboard with on-the-screen panel. * Add text mentioning NetBSD License. * Implement Aspect-ratio correction. * Figure out why in-game GUI call consumes so many seconds. * README. :)
comment:9 by , 19 years ago
It's v3 patch. :)
Changes: * Implemented basic pre-launcher / splash screen, and gamma ramp. Pre-launcher is not finished, however. * Improved button event handling. Now polls button event from queue. * Other slight changes...
Btw, how to change FMOPL / sample rate settings from pre- launcher without modify common ScummVM source code? Do I have to use commandline arguments?
comment:10 by , 19 years ago
I have no objections to the non-GP32 specific parts of the patch, so from my point of view this could be submitted to CVS now.
comment:11 by , 19 years ago
So I commited the patch. Won, in a couple days you will get CVS write access to continue your work. Thanks a lot and looking forward to collaborate with you officially :)
comment:12 by , 19 years ago
| Status: | new → closed |
|---|
comment:13 by , 6 years ago
| Component: | → Ports |
|---|
comment:14 by , 6 years ago
| Component: | Ports → Port: GP32 |
|---|
re-upload v1 patch