Re: [AD] Importing Amiga Allegro

[Peter Wang <novalazy@xxxxxxxxxx>]

On 2010-01-03, Arthur Ward <lists@xxxxxxxxxx> wrote:
> On 03/01/2010, you wrote:
> > 
> >> And double check the output of the above to see only the files which
> >> should be are added/modified. And finally:
> >> 
> >> svn ci -m"Added Amiga port...add other info here..."
> >
> > Please post a diff first.
> 
> Attached.  There is no makefile yet because we don't have cmake on the Amiga
> and thus far my releases have all been against 4.3.10, so I only upgraded
> to 4.4 in order to get the source into svn (the 4.3.10 tree seems to have
> disappeared).  I'm really submitting it mainly because it's been on my HD
> for 12 months and I want to get it into the public domain.

> Index: src/amiga/amouse.h
> ===================================================================
> --- src/amiga/amouse.h	(revision 0)
> +++ src/amiga/amouse.h	(revision 0)
> @@ -0,0 +1,33 @@
> +/*         ______   ___    ___
> + *        /\  _  \ /\_ \  /\_ \
> + *        \ \ \L\ \\//\ \ \//\ \      __     __   _ __   ___
> + *         \ \  __ \ \ \ \  \ \ \   /'__`\ /'_ `\/\`'__\/ __`\
> + *          \ \ \/\ \ \_\ \_ \_\ \_/\  __//\ \L\ \ \ \//\ \L\ \
> + *           \ \_\ \_\/\____\/\____\ \____\ \____ \ \_\\ \____/
> + *            \/_/\/_/\/____/\/____/\/____/\/___L\ \/_/ \/___/
> + *                                           /\____/
> + *                                           \_/__/
> + *
> + *      Amiga OS mouse module.
> + *
> + *      By Hitman/Code HQ.
> + *
> + *      See readme.txt for copyright information.
> + */
> +
> +#ifndef AMOUSE_H
> +#define AMOUSE_H
> +
> +/* Variable exported from graphics.c */
> +
> +extern int gCustomMouse;
> +
> +void mouse_handle_buttons(int aCode);
> +
> +void mouse_handle_move(int aX, int aY);
> +
> +void mouse_handle_wheel(int aZ);
> +
> +void mouse_set_hardware_imagery(int aX, int aY);
> +
> +#endif /* ! AMOUSE_H */

Internal symbols should begin with _, and preferably _al.
For Amiga-specific symbols use _al_amiga_* or similar.


Some minor things:

- It would be nice to stick to the standard formatting style, if you wouldn't
  mind running an auto-formatter over your code.  If not, fine.  We're
  traditionally lenient in the platform-specific directories, but it *has* left
  a mess in some directories.

- Your files are marked svn:executable.

- Some functions are really, really long.  You're likely the only one who will
  maintain this code so I don't really care, just commenting.

Peter


> Index: src/amiga/afile.c
> ===================================================================
> --- src/amiga/afile.c	(revision 0)
> +++ src/amiga/afile.c	(revision 0)
> @@ -0,0 +1,484 @@
> +/*         ______   ___    ___
> + *        /\  _  \ /\_ \  /\_ \ 
> + *        \ \ \L\ \\//\ \ \//\ \      __     __   _ __   ___ 
> + *         \ \  __ \ \ \ \  \ \ \   /'__`\ /'_ `\/\`'__\/ __`\
> + *          \ \ \/\ \ \_\ \_ \_\ \_/\  __//\ \L\ \ \ \//\ \L\ \
> + *           \ \_\ \_\/\____\/\____\ \____\ \____ \ \_\\ \____/
> + *            \/_/\/_/\/____/\/____/\/____/\/___L\ \/_/ \/___/
> + *                                           /\____/
> + *                                           \_/__/
> + *
> + *      Helper routines to make file.c work on Unix (POSIX) platforms.
> + *
> + *      By Michael Bukin.

Update this.

> +/* Driver info arrays used for by the system driver */
> +
> +extern _DRIVER_INFO _gfx_driver_list[];
> +extern _DRIVER_INFO _digi_driver_list[];
> +extern _DRIVER_INFO _midi_driver_list[];
> +extern _DRIVER_INFO _keyboard_driver_list[];
> +extern _DRIVER_INFO _mouse_driver_list[];
> +extern _DRIVER_INFO _joystick_driver_list[];

You would be better not to do this.

> +
> +static void system_exit();

Functions without arguments should be declared (void).

> +
> +unsigned int al_get_sobj_version()
> +{
> +	return((1 << 16) | 2);
> +}

Try not to introduce al_* symbols as we are using that for the Allegro 5
branch.