Opened 15 years ago

Closed 14 years ago

Last modified 11 months ago

#8415 closed patch

AmigaOS 4 changes

Reported by: SF/capehill Owned by: fingolfin
Priority: normal Component: Port: AmigaOS4
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)

patch2.diff.bz2 (5.6 KB ) - added by SF/capehill 15 years ago.
AmigaOS 4 changes, a new attempt
AmigaOS4_compiling.txt (496 bytes ) - added by SF/capehill 15 years ago.
How to build AmigaOS 4 version (with Cygwin)
patch3.diff (17.5 KB ) - added by SF/capehill 15 years ago.
AmigaOS 4 changes, 3rd attempt
patch4.diff (20.3 KB ) - added by SF/capehill 14 years ago.
AmigaOS 4 changes
amigaos4-fs.cpp (8.3 KB ) - added by SF/capehill 14 years ago.
AmigaOS 4 filesystem backend
patch5.diff (12.0 KB ) - added by SF/capehill 14 years ago.
AmigaOS 4 changes (excluding 1 new file, the filesystem backend)

Download all attachments as: .zip

Change History (22)

comment:1 by sev-, 15 years ago

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.

by SF/capehill, 15 years ago

Attachment: patch2.diff.bz2 added

AmigaOS 4 changes, a new attempt

comment:2 by SF/capehill, 15 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 fingolfin, 15 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 SF/capehill, 15 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 sev-, 15 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 SF/capehill, 15 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.

comment:7 by SF/capehill, 15 years ago

upload test...

by SF/capehill, 15 years ago

Attachment: AmigaOS4_compiling.txt added

How to build AmigaOS 4 version (with Cygwin)

by SF/capehill, 15 years ago

Attachment: patch3.diff added

AmigaOS 4 changes, 3rd attempt

comment:8 by fingolfin, 14 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 fingolfin, 14 years ago

Owner: set to fingolfin

comment:10 by fingolfin, 14 years ago

Any news on this? I'd like to get this patch integrated really soon...

by SF/capehill, 14 years ago

Attachment: patch4.diff added

AmigaOS 4 changes

comment:11 by SF/capehill, 14 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 fingolfin, 14 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!

comment:13 by SF/capehill, 14 years ago

Patch5 (cvs diff) + filesystem backend. I hope these work better.

by SF/capehill, 14 years ago

Attachment: amigaos4-fs.cpp added

AmigaOS 4 filesystem backend

by SF/capehill, 14 years ago

Attachment: patch5.diff added

AmigaOS 4 changes (excluding 1 new file, the filesystem backend)

comment:14 by fingolfin, 14 years ago

Status: newclosed

comment:15 by fingolfin, 14 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 digitall, 11 months ago

Component: Port: AmigaOS4
Note: See TracTickets for help on using tickets.