Re: [AD] file interface fopen

[Matthew Leverton <meffer@xxxxxxxxxx>]

On Sat, Dec 18, 2010 at 3:21 AM, Thomas Fjellstrom
<tfjellstrom@xxxxxxxxxx> wrote:
> I think it'd be cleaner to provide a al_create_file_vt() or some such, that
> passes in the vtable. Instead of all that messing with state junk.
>
I agree, but that's actually a different issue. (That is, it could be
useful with or without my proposal.) You'd have:

al_fopen(filename, mode);
al_fopen_vt(filename, mode, &vt);

But I think I like this better than the al_create_file():

al_fopen_userdata(ptr);
al_fopen_userdata_vt(ptr, &vt);

The latter one is what the memfile could use to avoid the state. Also,
it could eliminate the need for the al_set_file_userdata function.

On Sat, Dec 18, 2010 at 5:39 AM, Peter Wang <novalazy@xxxxxxxxxx> wrote:
> ALLEGRO_FS_INTERFACE is public and documented.
>
So is ALLEGRO_FILE_INTERFACE... I was just talking about the FS
process in general, which I haven't looked at.

> ALLEGRO_FILE is documented as being opaque, but that could be considered
> a mistake from earlier in the development process.  If we change the
> documentation, would that solve the problem for you?
>
I'm not a big fan of requiring somebody to use struct inheritance, as
is currently required by the ALLEGRO_FILE_INTERFACE part. Of course,
that comes down to personal preference, but I find my proposed version
easier to follow and understand than the current implementation.

The current fi_fopen + struct inheritance could be considered the
internal Allegro hook (for physfs and stdio), not meant for public
usage. The fi_fopen2 could be named something like fi_fopen_f or
fi_fopen_userdata and be meant for external user code.

And yes, if something like the above doesn't make its way into
Allegro, then the ALLEGRO_FILE type must be made public. I think it
sets a bad precedent to encourage people to dig into the internal
headers, especially when writing user code. But my preference would be
to add al_fopen_userdata() and al_get_file_userdata().

--
Matthew Leverton