Re: Collider and Mover code

[Gervase Lam]

> Date: Sun, 26 Jan 2003 10:32:56 +0100
> From: David Philippi <address@hidden>
> Subject: Re: Collider and Mover code

> Mover will never take any value by reference unless you change the
> signat= ure=20
> to Collider & collider. If you want to allow changeable colliders as
> well= as=20
> temporaries you'll need to methods:
>
> virtual void update(const Vector& move, const Collider& collider) const
> =3D= 0
> virtual void update(const Vector& move, Collider& collder) [ =3D0 ]

I have given this some thought and think that just having the const 
Collider& version is good enough.  The non-const version isn't really 
needed as Colliders are supposed to be really simple things.  All it does 
is return true or false depending on whether the Collider collides with 
something at a position vector (x, y) or not.  No feedback or state 
information needs to be passed back.

The initial problem I had with having the above two methods instead of one 
is that they both do the same thing.  Doing it the two methods way would 
mean duplicate code.

So, is there a way to stop duplicate code being written?  Could the two 
methods call a common method?

I'll probably still think about this situation every now and again.  A 
good solution could come in handy in the future.

While thinking about how to get around this, I realised that the Movers 
I've coded don't need to be templates at all.  At the moment, I've got:

class UprightPinguCollider : public Collider
{
// Function Object declarations... blah... blah... blah...
};

template <typename ColliderFunctor> class Mover
{
public:
  virtual void update(const Vector& move, ColliderFunctor collider) = 0;
};

template <typename ColliderFunctor> class LinearMover : public Mover
{
public:
  void update(const Vector& move, ColliderFunctor collider);
};

LinearMover<UprightPinguCollider> mover;

mover.update(move_vector, UprightPinguCollider());

Why does Mover need to be a template?  The following is better:

class Mover
{
public:
  virtual void update(const Vector& move, Collider collider) = 0;
};

class LinearMover : public Mover
{
public:
  void update(const Vector& move, Collider collider);
};

LinearMover mover;

mover.update(move_vector, UprightPinguCollider());

> > I've changed the counter i to an int.  I assume that the expression (i
> > =
>
> <
>
> > move_length) would promote i to a float before doing the comparison.
>
> Well, you're partially right. It would be promoted to an float for
> the=20 comparison so using float is better but I think changing
> move_length to i= nt=20
> does even more good. We are operating with integer steps and so we're
> goi= ng=20
> to loose the precision of float anyway - why not right at the beginning?

OK.  I'll do that.

> Are those compiler errors or linker errors? They look more like linker
> er= rors,=20
> so you should make sure that the .o of upright_pingu_collider comes
> befor= e=20
> bomber.o in the linking stage (or rather the colliders.a must be before
> t= he=20
> actions.a).

I moved the colliders directory right to the top of Makefile.am in 
Games/Pingus/src and still got the same problem after doing a make clean.  
I thought it might be a namespaces issue.  So I got rid of Collider 
namespaces.  I still get the problem.

Here is a more detailed error log:

|| Making all in colliders
|| make[1]: Entering directory 
`/home/gervase/pingus/cvs/Games/Pingus/src/colliders'
|| source='upright_pingu_collider.cxx' object='upright_pingu_collider.o' 
libtool=no \
|| depfile='.deps/upright_pingu_collider.Po' 
tmpdepfile='.deps/upright_pingu_collider.TPo' \
|| depmode=gcc3 /bin/sh ../../depcomp \
|| c++ -DHAVE_CONFIG_H -I. -I. -I../..   -I/usr/include/libxml2/libxml 
-I/usr/include/libxml2  -g -O2 -c -o upright_pingu_collider.o `test -f 
upright_pingu_collider.cxx || echo './'`upright_pingu_collider.cxx
|| rm -f libpingus_colliders.a
|| ar cru libpingus_colliders.a upright_pingu_collider.o 
|| ranlib libpingus_colliders.a
|| make[1]: Leaving directory 
`/home/gervase/pingus/cvs/Games/Pingus/src/colliders'
|| Making all in actions
|| make[1]: Entering directory 
`/home/gervase/pingus/cvs/Games/Pingus/src/actions'
|| source='bomber.cxx' object='bomber.o' libtool=no \
|| depfile='.deps/bomber.Po' tmpdepfile='.deps/bomber.TPo' \
|| depmode=gcc3 /bin/sh ../../depcomp \
|| c++ -DHAVE_CONFIG_H -I. -I. -I../..   -I/usr/include/libxml2/libxml 
-I/usr/include/libxml2  -g -O2 -c -o bomber.o `test -f bomber.cxx || echo 
'./'`bomber.cxx
|| rm -f libpingu_actions.a
|| ar cru libpingu_actions.a angel.o basher.o blocker.o boarder.o bomber.o 
bridger.o climber.o digger.o drown.o exiter.o faller.o floater.o jumper.o 
laser_kill.o miner.o rocket_launcher.o slider.o smashed.o splashed.o 
superman.o teleported.o waiter.o walker.o 
|| ranlib libpingu_actions.a

[...lots of enter and leaving directories with no building...]

|| c++  -g -O2   -o pingus  action_button.o action_data.o action_holder.o 
algo.o alpha_button.o blitter.o button_panel.o capture_rectangle.o cheat.o 
client.o col_map.o config.o config_xml.o console.o counter.o credits.o 
cursor.o debug.o debug_stream.o demo_player.o demo_recorder.o 
demo_session.o direction.o display.o display_graphic_context.o exit_menu.o 
fade_out.o fonts.o fps_counter.o game_counter.o game_session.o 
game_session_result.o game_time.o global_event.o globals.o goal_manager.o 
groundtype.o gui_obj.o gui_screen.o html_browser.o hurry_up.o 
input_debug_screen.o indexed_canvas.o intro.o layer_manager.o level_desc.o 
level_interrupt.o level_result.o loading.o main_menu.o menu_background.o 
multiline_text.o option_menu.o path_manager.o pingu.o pingu_action.o 
pingu_action_factory.o collider.o pingu_enums.o pingu_holder.o 
pingu_info.o pingu_map.o pingus_counter.o pingus_counter_bar.o 
pingus_error.o pingus_main.o pingus_map_manager.o pingus_menu.o 
pingus_menu_manager.o pingus_resource.o pingus_sub_menu.o pla
|| ield.o playfield_view.o plf.o plt_xml.o prefab.o range.o 
res_descriptor.o resource_modifier.o result.o screenshot.o 
screen_manager.o screen_ptr.o server.o server_event.o smallmap.o 
smallmap_image.o sound.o sound_dummy.o sound_real.o specs_reader.o 
spot_map.o sprite.o string_converter.o surface_button.o system.o 
target_provider.o theme.o theme_selector.o time_display.o timer.o 
true_server.o vector.o view.o wav_provider.o world.o worldobj.o 
worldobj_data.o worldobj_data_factory.o xml_helper.o xml_plf.o xml_pdf.o 
-lclanGUI -lclanPNG -lclanJPEG -lclanMikMod -lclanSound -lclanDisplay 
-lclanApp -lclanCore -lHermes -lpthread  -L/usr/lib -lxml2 -lz -lm 
caimagemanipulation/libcaimagemanipulation.a 
colliders/libpingus_colliders.a editorobjs/libpingus_editorobjs.a 
worldobjs/entrances/libpingus_worldobjs_entrances.a 
worldobjsdata/libpingus_worldobjsdata.a worldobjs/libpingus_worldobjs.a 
worldobjsdata/libpingus_worldobjsdata.a 
worldobjs/entrances/libpingus_worldobjs_entrances.a 
editorobjs/libpingus_editorobjs.a actions
|| ibpingu_actions.a particles/libpingus_particle.a 
editor/libpingus_editor.a worldmap/libpingus_worldmap.a 
gui/libpingus_gui.a input/libpingus_input.a 
input/axes/libpingus_input_axes.a input/buttons/libpingus_input_buttons.a 
input/pointers/libpingus_input_pointers.a 
input/scrollers/libpingus_input_scrollers.a -lclanGUI -lclanPNG -lclanJPEG 
-lclanMikMod -lclanSound -lclanDisplay -lclanApp -lclanCore -lHermes 
-lpthread  -L/usr/lib -lxml2 -lz -lm
|| actions/libpingu_actions.a(bomber.o): In function 
`Actions::Bomber::update()':
actions/bomber.cxx|96| undefined reference to 
`UprightPinguCollider::UprightPinguCollider(bool)'
actions/bomber.cxx|98| undefined reference to 
`UprightPinguCollider::UprightPinguCollider(bool)'
|| actions/libpingu_actions.a(bomber.o): In function 
`std::basic_streambuf<char, std::char_traits<char> >::pbackfail(int)':
|| 
/usr/include/g++-v3/bits/std_streambuf.h(.gnu.linkonce.t._ZN6Movers11LinearMoverI20UprightPinguColliderE6updateERK6VectorS1_+0xed):
 
undefined reference to `UprightPinguCollider::~UprightPinguCollider()'
|| 
/usr/include/g++-v3/bits/std_streambuf.h(.gnu.linkonce.t._ZN6Movers11LinearMoverI20UprightPinguColliderE6updateERK6VectorS1_+0x10a):
 
undefined reference to `UprightPinguCollider::~UprightPinguCollider()'

Thanks,
Gervase.