Re: [AD] fshook changes

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]


On 2009-01-22, Thomas Fjellstrom <tfjellstrom@xxxxxxxxxx> wrote:
> On January 22, 2009, Peter Wang wrote:
> > On 2009-01-22, Thomas Fjellstrom <tfjellstrom@xxxxxxxxxx> wrote:
> > > On January 22, 2009, Peter Wang wrote:
> > > > I didn't understand what you said then. The sentence with six commas.
> > > >
> > > > What would addons need that header?  Isn't it meant to be internal?
> > >
> > > Well, it was supposed to be initially. but addons are permitted to use
> > > them are they not?
> >
> > Preferably not.  If they do, it usually indicates a failing of the design.
> 
> Look at the memfile addon, is that a failing of design? how about Allegro GL in 
> a4? They both need internals. How would you suggest the memfile addon be fixed 
> to not depend on any internals, while still providing a new type of file 
> handle?

You shouldn't need to include aintern_fshook.h, nor define a macro
called ALLEGRO_LIB_BUILD when it is clearly not true.

(aintern_memory.h shouldn't be included either, but we still need to add
al_malloc and al_free functions.)

> > In this case, the whole point for the existence of fshooks is so they
> > can be extended by the user.  That means it should be possible to do
> > without reaching into internal headers.  So the vtables (or whatever) do
> > need to be moved out into the public headers, though they don't need to
> > be included by default.
> 
> I disagree. The hook vtables should not be globally public.

Making your own types of "fs" is the normal mode of operation for
fshooks.  You aren't doing anything dirty or unsupported or liable to
change.  Whatever you need to achieve that should be part of the public
interface and documented and stable.  As it is right now, that happens
to be the fshook vtables.

> > Actually, they *will* be stuck, for ABI compatibility.  If you want to
> > be able to change the vtables then those macros need to be functions.
> > But that's what the fshook layer already *is* anyway.
> 
> If you want to bring back the functions to set and get the vtable, they aren't 
> stuck. I just figured it looked odd against the other bare vtables in allegro 
> so I decided to remove them (even though they should probably all use them...)

IIRC the number of getters/setters was pretty ridiculous, but maybe I am
thinking of something else.

Peter





Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/