#8871 closed patch
Nintendo Wii port
| Reported by: | dhewg | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | Port: Wii |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
attached you will find a diff for a wii port against svn trunk
this is the very same code i used to build the binary posted today on http://hbc.hackmii.com/download/
current build requirements: - devkitppc cvs - libogc cvs - libfat cvs
compared to the first binaries i made public i did not have to use any patches, all requirements have been commited to the official devkitppc cvs. svpe fixed libfat to be thread safe on the last minute (thanks buddy!) and eg. monkey island 3 is playable now.
additional notes: - all crashes (known to me) on prior binaries i made available have been fixed in devkitpro/libogc/libfat - wiimote code has been added, i consider the relevant libraries stable by now - libogc currently lacks IR pointer smoothing. i added a small function (coded by marcan) to do so, this should be replaced once the functionality is in libogc - sd reading is slow, because libfat currently requests each cluster (512 bytes) with a single sd command. this results in sound stutter and dropped frames on some sequences in various games - libfat has been ported to the gamecube as well (using a sd gecko adapter). wiimote support is the only wii specific feature used, thus a gamecube port is easily possible. i already added defines to wii specific code parts, but did not try to compile a gamecube version (lack of time) - the current approach to draw a frame is not optimal (buffers are copied)
i'm sorry it took so long to post this diff. i had the honour of working with very talented people on "the homebrew channel" project for the wii, which alone took much of my free time. we found and reported/fixed problems within devkitppc and libogc, and scummvm gained features as well as stability from those fixes. i hope this was worth the wait.
Ticket imported from: #1971285. Ticket imported from: patches/976.
Attachments (5)
Change History (27)
by , 16 years ago
| Attachment: | scummvm-wii-take1.diff added |
|---|
comment:2 by , 16 years ago
Thanks a lot for your patch. I'll give it a brief look now, and I am sure Eugene will want to do the same.
P.S.: I got a Gamecube as a present recently, so I'd certainly be curious about a GC port, but only if it works w/o modding (maybe that's not possible, though ;)
comment:3 by , 16 years ago
| Owner: | set to |
|---|
comment:4 by , 16 years ago
Some remarks on the patch (please don't take them to negative -- overall, the port looks good, I am just trying in my way to make it even better, so that it will be less work to maintain the port if/once it gets official, for all involved parties)
* Please don't modify OSystem::getFilesystemFactory() in common/system.cpp -- rather, overload that method. See also the TODO comment just above it, in line 128 of common/system.cpp (but it should be trivial for you to adapt to that, I think :-)
* Please follow our code formatting guidelines, as described on <http://wiki.scummvm.org/index.php/Code_Formatting_Conventions> -- in particular tabs instead of spaces for indention, and no spaces before parentheses in funcs (i.e. "foo()" instead of "foo ()")
* backends/platform/wii/gecko_console.cpp is problematic -- this file is not under the GPL and also has a 3rd party author. What is the file used for?
* Same problem for marcan's code in backends/platform/wii/osystem_events.cpp -- we can't just insert code with a different license in our code, marcan needs to relicense / dual license his good in order for us to use it, I think.
* WiiFilesystemNode does not seem to implement isReadable && isWritable properly, which could cause problems in the future. Are those TODOs in there on purpose, or just an oversight?
Overall the patch looks pretty good already!
comment:5 by , 16 years ago
- how meta.xml is used? If it is a run time thing, then more appropriate place for it would be in dists/wii directory. - It would be better to use #ifndef _WII_OSYSTEM_H_ instead of _OSYSTEM_H_ in wii/osystem.h to avoid oitential name clashes - Use similar WII_ prefix in other wii-specific includes - gecko_console.cpp has a copyleft license, so I don't see any problems with it - same applies to marcan's code. It is a copyleft license - I beg you not to use any port-specific alterations such as ScummVM-Wii. It is a "Wii port of ScummVM", not something different
comment:6 by , 16 years ago
Actually, that license is not a "copyleft" license (at least it does not match the definition given for Copyleft in e.g. Wikipedia). But I just checked and discovered that it is the zlib license, which is indeed GPL compatible: <http://en.wikipedia.org/wiki/Zlib_License>. So indeed, no problem there. Fine :)
comment:7 by , 16 years ago
thanks for your comments!
i addressed the mentioned issues, attached you'll find take2
* Please don't modify OSystem::getFilesystemFactory() in common/system.cpp done. i already overloaded that one, but forgot to remove the code in common/system.cpp
* backends/platform/wii/gecko_console.cpp / smooth_ir() yes its the zlib license and compatible to gpl. unchanged
* WiiFilesystemNode does not seem to implement isReadable && isWritable fixed, just an oversight
* how meta.xml is used? yes, its runtime data for the homebrew channel (as is icon.png). moved to dists/wii
* #ifndef _WII_OSYSTEM_H_ instead of _OSYSTEM_H_ good point, i changed all defines is wii specific header files
* "Wii port of ScummVM" instead of "ScummVM-Wii" done
* Please follow our code formatting guidelines done. but i still feel pain, just so you know...
about gecko_console.cpp: is a so called devoptab device for the used newlib libc. with this code stdout is redirected over the usbgecko device. one can read all debug messages via a simple "cat /dev/ttyUSB0" on the pc side. the code does not block and works without that adapter too. see http://www.usbgecko.com/
and yes, it is possible to boot homebrew on a unmodded gamecube, see http://www.gc-linux.org/wiki/SDload ;)
File Added: scummvm-wii-take2.diff
comment:8 by , 16 years ago
Besides several nitpick whitespace corrections I see it as ready for inclusion.
One question, though. Is there some particular reason why you use s16, u16 instead of ScummVM's int16 and uint16? (other variable widths apply)
comment:9 by , 16 years ago
Do you really need all these in portdefs??? Ideally, none of them should be neede, I'd be curious to know which is required for which of our engines etc. source files:
+#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <stdarg.h> +#include <malloc.h> +#include <math.h> +#include <time.h> +#include <ctype.h> +#include <assert.h>
comment:10 by , 16 years ago
Err, and just to clarify: I like the patch so far, these are minor tidbits, please don't be discouraged ! Maybe my last comment sounded too negative :)
comment:11 by , 16 years ago
just mention all tidbits, i do understand every single issue. even if its only some whitespaces i missed, its a huge code base and there is no need to even argue about eg. a common coding style. no offense taken at all ;)
about the s16, u32 etc types: these are defined in libogc and are used in all dependent libraries. i just got used to those, its pretty much common for all gc/wii projects.
about the includes in portdefs.h: i tried compiling without those, but time.h was the only one i could kick out: ../../../common/str.h:145: error: 'assert' was not declared in this scope ../../../common/stream.h:299: error: 'SEEK_SET' was not declared in this scope ../../../common/stream.h:479: error: 'memcpy' was not declared in this scope ../../../common/stream.h:421: error: 'free' was not declared in this scope ../../../engines/scumm/actor.cpp:236: error: 'atan2' was not declared in this scope ../../../engines/scumm/detection.cpp:63: error: 'bsearch' was not declared in this scope ../../../engines/scumm/debugger.cpp:728: error: 'qsort' was not declared in this scope ../../../engines/scumm/debugger.cpp:53: error: 'va_start' was not declared in this scope
i attached take3 (mostly whitespace fixes) File Added: scummvm-wii-take3.diff
comment:12 by , 16 years ago
OK, but: those #includes are automatically there if you do NOT define NONSTANDARD_PORT. Do you really need portdefs.h ? It seems to me that you mostly need it for some tiny #defines, which could also be moved to the WII part of scummsys.h...
I am trying to compile your port now. A README explaining how to do that, resp. what you need, would be nice. I installed DevKitPPC, including libogc, but it complains about missing fat.h & gba_types.h-- so i guess I need the GC libfat. But where should I put those files? the Makefile sets no include path for libfat. Hu?
I noticed that you redefine the clean&dist-clean targets in your Makefile. Is that on purpose? You might want to instead *add* to those targets. you can do that like this:
clean: clean-WII-SPECIFIC clean-WII-SPECIFIC: YOUR COMMANDS HERE
That way, clean will do what it normally would do, *plus* your code.
comment:13 by , 16 years ago
ok, next try ;)
* NONSTANDARD_PORT you're right, it wasn't needed. dropped it
* Makefile targets changed. but now i am getting an error on `make clean`. i couldn't figure out where this is coming from: tools/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ sound/softsynth/mt32/ make: tools/: Command not found make: *** [clean] Error 127
i also adopted the wiimote code for the most recent libogc cvs File Added: scummvm-wii-take4.diff
comment:14 by , 16 years ago
i fixed some bugs in the wii port, see the changelog here: http://wiibrew.org/index.php?title=Homebrew_apps/ScummVM
devkitPPC r15 has been released, so you should be able to compile it with just one "make" File Added: scummvm-wii-take5.diff
comment:15 by , 16 years ago
Looks much better.
Some notes for TODO (which could be considered after inclusion)
- setShakePos() is not implemented. It is particularly used in many SCUMM games - It looks that you don't have cursor scaling implemented. It is used by some HE games - Would be not bad to describe GAMECUBE define, or even give it more meaningful name like GAMECUBE_PAD - Would be not bad to have controller selectable at run time, not at compile time. See http://wiki.scummvm.org/index.php/GUI_Themes/Specs#Backend-specific_theme_additions on how to add your backend-specific GUI widgets - You seem to miss a virtual keyboard. Yet another port which will benefit from our GSoC task - I recommend to drop MPEG2 support, pretty soon we will declare it obsolete
comment:16 by , 16 years ago
@dhweg: great, will try out the new devkit when I am bac hom
@sev: As you said, most of these could indeed be resolved after inclusion. As for the "We will drop MPEG2 support soon" -- uhm, we will? I think we really need to discuss that bit, before we advice people to drop support for it...
comment:17 by , 16 years ago
Compiles like a charm! Though I would add -Wno-unknown-pragmas to the CXXFLAGS. Maybe just use this line from our regular Makefile:
CXXFLAGS+= -Wno-long-long -Wno-multichar -Wno-unknown-pragmas -Wno-reorder
From my point of view this is essentially ready to be commited. Though one day, I'd like to discuss the possibility to change the build system to use our regular build system. So that a wii version can be compiled with ./configure --host=wii && make
comment:19 by , 16 years ago
there are still some minor things missing, you pointed out some of those. but i believe the port is in a pretty good state now, from the reports i've been getting there are some hickups, but no crashes. most (if not all) games are playable, so: i totally vote for commiting it! ;)
comment:20 by , 16 years ago
| Status: | new → closed |
|---|
comment:22 by , 6 years ago
| Component: | → Port: Wii |
|---|
diff against trunk