Thursday, April 29, 2021

UsingSprites was a mistake. Wasn't it ?

 While refactoring the animation editor, I tasted a new design pattern as an alternative to the usual Singleton pattern: the Using* anti-pattern. The idea was simple: for every item that needs to be there one and only one time, there is a class who has the sole responsibility for holding the variables describing that item. Let's take for instance the hardware sprites of the Nintendo DS. We have an array of 'sprites', an array of 'rotations' and a z-order list.

The Using* anti-pattern said "these 3 should be static members of UsingSprites, and any code who needs to access them (read or write) will derive from UsingSprites". What it guarantees, is that if something messed up with * global, shared state, then that something is in one of the classes Using*.

The one nice thing, is that code in those sub-classes can access the array quite directly: the location of UsingSprites::sprites is stored next to the functions that need it and it just needs a ld r*, [pc, offset] to be ready to use it. No additional pointer dereferencing required. This is the very same as accessing e.g. the address of a constant string (ARM thumb code doesn't seem to have mov reg, imm32 instruction).

What I hadn't foreseen when I decided to generalise that to the whole game engine, is that a) there would be so many Using* for simple classes such as an animation and b) that they would all introduce a virtual table where the pointers to virtual methods for that class are stored.

The issue with that is that when you have multiple inheritance, you need one vptr (pointer to a virtual table) per super-class. That means every GobAnim instance must dedicate 5 full-sized pointers to virtual tables in addition to the 'real' data of the object.

Hopefully, it turns out to be a worst-case. GobState has 4 superclasses and most of the state-machine only one or two.

When looking at that inheritance graph again, I wonder if Using* is the right one to blame. iReport is there just to bring action/step/report/diagnose functions into the local namespace, but they are actually just static functions manipulating no particular state. Same goes for iAnimUser. iReport is so omnious that it could really be dropped and the functions just promoted to global namespace just like printf. iAnimUser could just be a namespace and not mess up with the inheritance, as it just provides constants and helper inline functions with, again, no dedicated state.


1 comment:

PypeBros said...

there is boost::noncopyable. it works like my 'NOCOPY' macro, but through class inheritance. It just doesn't define destructor to be virtual.

And unfortunately, that means -Weffc++ complains about every class that uses it. a pragma GCC disagnostic ignore("-Weffc++") can prevent production of 'warning: base class '...' has non-vritual destructor pointing noncopyable.hpp, but it cannot prevent the same message at the .cpp where I try to derive from noncopyable.