Better module initialization

Copied from bug 4 in old bug database.

grelminar: writing MM_foo code for loading, unloading, attaching, and detaching can be tedious and error-prone. there should be a better way to do this.

sample code for one possible scheme is in the attached file. I'm not completely happy with it, mostly because of the gross lack of type safety.

{{{

#include "asss.h"

struct autowarp_chunk { i16 x; i16 y; char arena[16]; };

local Iarenaman aman; local Igame game; local Imapdata *mapdata;

local int pdkey, adkey;

typedef struct pdata { int stuff1; } pdata; typedef struct adata { const char *stuff2; } adata;

local void region_cb(Player p, Region rgn, int x, int y, int entering) { const struct autowarp_chunk *aw; int awlen;

if (!entering)
    return;

if (!mapdata->RegionChunk(rgn, MAKE_CHUNK_TYPE(rAWP), (const void **)&aw, &awlen))
    return;

if (awlen == 4 || (awlen == 20 && aw->arena[0] == '\0'))
{
    /* in-arena warp */
    Target t;
    t.type = T_PLAYER;
    t.u.p = p;
    game->WarpTo(&t, aw->x < 0 ? x : aw->x, aw->y < 0 ? y : aw->y);
}
else if (awlen == 20)
{
    /* cross-arena warp */
    aman->SendToArena(p, aw->arena, aw->x < 0 ? x : aw->x, aw->y < 0 ? y : aw->y);
}

}

typedef intptr_t msv;

#define G_INT(ptr, iid) (msv)1, (msv)(&ptr), (msv)(iid), #define G_CB(ptr, cbid) (msv)2, (msv)(&ptr), (msv)(cbid), #define P_DATA(ptr, len) (msv)3, (msv)(&ptr), (msv)(len), #define A_DATA(ptr, len) (msv)4, (msv)(&ptr), (msv)(len),

// similar for arena callbacks, global commands, arena commands, player // persistent data, arena persistent data, globally registered // interfaces, arena registered interfaces

#define MODULE(name) EXPORT const msv name ## _module_stuff[] = { #define MODULE_END() (msv)0 };

MODULE(autowarp) G_INT(aman, I_ARENAMAN) G_INT(game, I_GAME) G_INT(mapdata, I_MAPDATA)

G_CB(region_cb, CB_REGION)

P_DATA(pdkey, sizeof(pdata))
A_DATA(adkey, sizeof(adata))

MODULE_END()

}}}

numpf:

This looks like it fixes an old method of declaring stuff. The MM_autowarp in the current version of funky/autowarp.c looks similar to the attached file. If this is still an issue, can you be more specific about how the current method is bad? If not, close please.

Smong:

I don't think it's a good idea to use macros for this. It doesn't let you use arbitrary C code during the load, unload, etc actions. I can only see it working for the simplest of modules, which could (should?) be written in python.

grelminar:

Huh? The attached file looks nothing like any current or former way of writing module init code. Current: {{{ EXPORT int MM_autowarp(int action, Imodman mm, Arena arena) { if (action == MM_LOAD) { aman = mm->GetInterface(I_ARENAMAN, ALLARENAS); game = mm->GetInterface(I_GAME, ALLARENAS); mapdata = mm->GetInterface(I_MAPDATA, ALLARENAS); if (!aman || !game || !mapdata) return MM_FAIL; mm->RegCallback(CB_REGION, region_cb, ALLARENAS); return MM_OK; } else if (action == MM_UNLOAD) { mm->UnregCallback(CB_REGION, region_cb, ALLARENAS); mm->ReleaseInterface(aman); mm->ReleaseInterface(game); mm->ReleaseInterface(mapdata); return MM_OK; } return MM_FAIL; } }}}

Proposed: {{{ MODULE(autowarp) G_INT(aman, I_ARENAMAN) G_INT(game, I_GAME) G_INT(mapdata, I_MAPDATA)

G_CB(region_cb, CB_REGION)

P_DATA(pdkey, sizeof(pdata))
A_DATA(adkey, sizeof(adata))

MODULE_END() }}}

grelminar: @Smong: it's easily extensible to allow arbitrary actions. Just add a GENERIC_INIT macro that takes two void(*)(void) function pointers, one that gets called on load, one on unload. Or maybe four macros: ON_LOAD, ON_UNLOAD, ON_POSTLOAD, ON_PREUNLOAD, that take one function each.

The idea is to make the common case simple and declarative, and if you have to do fancy stuff, there's still a way to do it. The whole scheme might be too macro-happy for most people's tastes, though.

Smong: I suppose having callbacks for all the MM_ actions could work. I would definitely need custom detach and unload to clear timers and remove fake players.

This still leaves cleaning up timers and fakes to the programmer, so there's still room for error. Were you thinking of automatically cleaning those things up instead?

grelminar: The goal isn't to eliminate error completely (we're talking about C here, that's impossible), it's to make common things easier and make errors less likely.

The initial version would handle common tasks, and you'd have to write custom code for everything else. As more modules are written and converted, other init/cleanup patterns could be idenfied and macros could be added for them too.

grelminar: And as for the idea of automatically cleanup up fakes, timers, etc. when modules are unloaded: if we're willing to change a bunch of existing interfaces, it seems possible to have all the functions that allocate something or change some state that has to be cleaned up later to take a module pointer as an argument, and then get some notification when the module is unloaded, so the cleanup would really be automatic. That's a pretty significant change, though, and I think there's some benefit to implementing something similar to what's described in this bug, without going that far.

Comments (3)