#8415 closed patch
AmigaOS 4 changes
| Reported by: | SF/capehill | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Port: AmigaOS4 |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
AmigaOS 4 specific changes for ScummVM.
Ok, this is my first ever attempt to submit a patch so if there are problems please contact me. Thanks.
Generally any AmigaOS 4 modifications should been closed between #ifdef __amigaos4__ ... #endif
Ticket imported from: #1181544. Ticket imported from: patches/520.
Attachments (6)
Change History (22)
comment:1 by , 19 years ago
comment:2 by , 19 years ago
Ok. I wasn't able to kill PS2 last time so I try again ;)
I have gone through the code more carefully and I hope that it is more compatible with conventions now.
With little changes to configure script, I'm now able to build this on Cygwin with --host=ppc-amigaos.
This work is based on Hans-Joerg Friedens ScummVM 0.6.x port.
comment:3 by , 19 years ago
Thanks for your effort, capehill. However, before we can accept this into CVS -- did you talk to Hans-Joerg Friedens whether this is OK by him? We also need to resolve, for the credits/authors list, how this patch shall be attributed (maybe listing both you and Hans-Joerg Friedens as port maintainer?)
comment:4 by , 19 years ago
@fingolfin: I have talked with Hans-Jrg Frieden and he has no objections. You can mention both of us as maintainers, but I'm afraid Hans-Jrg is very occupied with his other projects at the moment. Anyway, we try to keep AmigaOS 4 port maintained :)
Ps. no ending 's' in his name. My typo.
comment:5 by , 19 years ago
Couple notes on new patch.
o You haven't fixed intentation in many places in amigaos4-fs.cpp. I.e now there is a wild mix of 4 spaces and tabs. Every indented line should start with tabs, no spaces at all. o Would be better to use backends/fs/fs.h in include instead of ../fs.h o Why not remove dprintf's at all and not just comment them out. Unless you have some speical reason of it.
Overall it looks in a good shape, but I'd recommend to wirte short README on how to build the port.
comment:6 by , 19 years ago
Again, thanks for the feedback. I'll attach a 3rd patching attempt + short instructions about how to build AmigaOS 4 version.
Tabs should be now fixed.
by , 19 years ago
| Attachment: | AmigaOS4_compiling.txt added |
|---|
How to build AmigaOS 4 version (with Cygwin)
comment:8 by , 19 years ago
OK, thanks.
* the patch for "configure" is semi-broken, it seems, apparently a windows versus unix newline issue?
* the configure patch reverts a change we made previously to CVS (to the way we handle CXXFLAGS). I'll just ignore that part of it.
* We need to credit your guys. backends/fs/amigaos4/amigaos4-fs.cpp should have mention the original author in the copyright part at the top (and the bogus 2002-2005 should be changed, too :-). Likewise, you need to be added to tools/credits.pl (from that file we'll generate all other things, like the AUTHORS file)
* Good thing you include AmigaOS4_compiling.txt, although right now I am not sure where to put that... it is rather short, maybe you can patch the README and docs/09.tex (adding the info there, where it belongs) ?
Other than that, this patch looks good to go.
comment:9 by , 19 years ago
| Owner: | set to |
|---|
comment:11 by , 19 years ago
Hello...I have be busy as a dog with 2 tails...
I hope that this patch solves current issues :)
- Readme changed - credits added - copyright year fixed - code checked to work with the CVS code - hopefully configure is ok too?
comment:12 by , 19 years ago
Patch doesn't apply:
patch: **** malformed patch at line 649: // capehill's note: casted to (int32*) because dispatch is declared so...
it looks as if you directly inserted a comment into the patch, not the source file, which won't work.
Even after removing that, it'll fail to apply, because again part of the configure script uses CRLF line ends -- you need to clean those up before making the diff!
by , 19 years ago
| Attachment: | patch5.diff added |
|---|
AmigaOS 4 changes (excluding 1 new file, the filesystem backend)
comment:14 by , 19 years ago
| Status: | new → closed |
|---|
comment:15 by , 19 years ago
Much better. I checked in your work into CVS, and mentioned it in the NEWS file.
Now, if you like to become a regular team member (i.e. with developer/ commit access to CVS), best thing to get it started is that you email me privately (fingolfin@scummvm.org should work, if not use my SF.net address), then I can talk to my co-lead Ender about it (a formality, mostly, so he, too, knows who's new to the team :-) etc.
comment:16 by , 5 years ago
| Component: | → Port: AmigaOS4 |
|---|
Thanks for your patch. Especially it is good that you explained obscure places with comments.
First, proper way to define backend-specific make rules is to use files build.rules and Makefile in your backend directory. See GP32 port as most recently reworked. The goal of this is to avoid clobbering of root project directory.
Next (minor) issue is file copyright headers in your new files. It would be better to sync them with current year :)
Then goes code formatting. It is good to have whole source tree consistent with our code formatting conventions (http://www.scummvm.org/documentation.php?view=conventions). Particularly denote your use of tabbed indentation and opening curly brackets also I spotted breaks of rule 4a here and there and crowded if() blah; clauses.
Instead of dprintf() you may consider use of our debug(level, format, ...) function which has run-time ability to filter out debug messages in contrast of compile-time of dprintf().
Also it is not clear why file backends/module.mk was completely replaced by your patch. Please, investigate.
For some reason your patch is based on outdated common/scaler/hq2x.cpp, common/scaler/hq3x.cpp, common/scummsys.h and common/stdafx.h, so you successfully break PS2 port. Is Amiga a PS2 killer? :) Please, update to current CVS.