Trouble compiling C Run-time under VS2010

Tiggertooth

A few weeks ago I discovered Spine, saw that it had a C++ interface and was very self-contained, and suggested to the art team we consider using it with our game engine. They got on board, I went off to GDC, got back, and now finally am ready to integrate the Spine C++ runtime into our C++ engine.

But the Spine C++ runtime is gone. So I decided to just use the C runtime. I'm having a lot of headaches with the C runtime. I'm looking for some help.

My engine is cross-platform C++. I do most of my original development in Visual Studio 2010 (on Windows) and then have a thin veneer on other platforms for the ports.

The C runtime isn't valid ANSI C code. It frequently declares variables in the middle of function, which isn't valid in ANSI C or C89. A lot of compilers allow this as an extension, particularly gcc, and the C99 rules allow for it, but native Visual Studio uses C89. So, it wouldn't compile.

Fine, I'll compile it as C++ code. But this doesn't work either. The structs throughout the code have const members, but no constructors. So these are invalid objects to instantiate. There's no way to instantiate a struct that has a const member without the const member being initialized.

Again, some compilers allow this as a workaround, or an extension. I haven't found a way to make this possible with Visual Studio. This is the heart of my current issue.

A prime example is I'm trying to derive my own class from the AttachmentLoader struct. But my constructor isn't valid because the AttachmentLoader has a const member (the vtable). If I remove the const members, things work fine. This is true throughout the code base.

But I'm loathe to manually remove the const in my copy of the C runtime codebase. Everytime there's an update, I'd have to do that all again. I was hoping to just wrap the C runtime without modification. (Or with very very little modification).

I could solve that issue by simply having our team settle on a single version of the runtime, and then re-sync to the latest code whenever I have time. But that isn't possible because the animation authoring tool (which I think is simply called Spine) automatically updates. So every day they run the risk of getting a new version, which could break with my version, which would then force me to have to re-get the source, make my modifications, and so on. I can use merging tools to help this process, but it still seems like unnecessary work. All because of some const members.

Questions:

1) Has anyone been able to get the current C Runtime to compile under Visual Studio 2010 C++ as C code?

2) Has anyone been able to get it to compile as C++ code (that's actually invokable)? Especially if trying to derive your own AttachmentLoader? (Again, with Visual Studio 2010.)

3) Is there anyway to turn off the auto-update feature of the Spine authoring tool?

My thoughts so far are that the authoring tool looks fantastic

it would take me a long time to come up with something as well presented as that. So it is a huge win for the team. But the C runtime feels really awkward...it is bending over backwards to be object-oriented, but trying to stay C. Yet it isn't very portable C. And the object-oriented stuff causes lots of headaches (the macros, loss of type-checking, const members that aren't instantiated). Essentially it is crying out to be C++ but isn't, yet isn't quite valid C either.

It feels like it would be better to make it all C++, with an optional C interface. C++ compiles virtually everywhere C does, and with a C interface. Languages that want C bindings could use the C interface of the lib. It wouldn't look much different than what you have today. But C++ environments would directly access the C++ classes. And on the inside, you get all the benefits of real object-oriented code.

I have a few other concerns with the C runtime, like using macros for memory allocation (rather than using a function pointer that can be registered, and defaults to malloc). As it is, the memory allocator has to be compiled in, which is problematic. But these issues are only worth worrying about if I can even get the code to compile. Which currently I can't.

Can anyone help?

Thanks,
Ken

Nate

Both C and C++ have different downsides.

I considered keeping the C++ runtime and providing a C wrapper but the result is more work, harder to maintain, and not as nice. Eg, every field in a class needs a get and set function, all vectors need get count, get item, add item, etc. None of that is terribly hard to write, but it's a lot just to wrap the C API and will grow as features are implemented in the C++ runtime. It also doesn't make for the most friendly C API. I didn't like throwing away my nice C++ code, but I'm not sure the downsides are worth having real OOP. The library is relatively small.

The C runtime is a single codebase to maintain and handles C/C++/ObjC. C isn't OO and some "tricks" are used. Surely it is debatable how clever this really is, but it works.

I would like to continue using C, but I'm open to improving it.

Ideally type safety is only lost in the game toolkit specific layer. The API that users actually use should be type safe.
The OO stuff could be reduced or removed. Mostly it is there for convenience, to easily to attach some game toolkit specific data to an object. We could use void* for that.
We could reduce the need for inheritance by extracting rendering to a separate class. This is done in spine-csharp, where SkeletonRenderer knows how to render attachments and the skeleton is only used a model. I think I like this better.
It could be made C89, though I have a feeling this is only really useful for VS and compiling as C++ works just fine there (and elsewhere).
Your issue with the structs having const members but no constructors is circumvented by using malloc. This works on all compilers. Use AtlasAttachmentLoader as a template.

The forced auto updates are an issue, I know. Auto update is relatively new and there is a ton of other work to be done. I've added a Trello card for the task. I'd like it to be possible to disable auto updates and also to run previous versions of Spine.

I agree that a function pointer would be better than macros. I'll keep the macros just because it helps show why code is allocating, but I'll add function pointers.

djr

I've forked the code on github and have a build working on visual studio express 2010. I've also patched a recent bug in the RegionAttachment_updateOffset code that is just updated.

I've created a pull request, I'm new to github (although I am an experienced developer) so I may have screwed it up. I've tried to keep it as clean as possible.

You can view the fork here: https://github.com/executionunit/spine-runtimes

Tiggertooth

Thanks for doing that work djr! Nicely done.

As an update, I've switched to compiling the source as C++, which solves some of the other issues. Because I'm extending (wrapping with my own C++ classes) some of the structures in Spine-C, I've had to make a few additional changes to make that work. Nate suggested using malloc to get around the issues of allocating const pointer member variables, but I've got my own memory management scheme, and using malloc at that level isn't an option.

So my solution to that was simply to add one line constructors to a few of the structs, protected with the __cplusplus preprocessor macro. Here's an example for the AttachmentLoader struct in AttachmentLoader.h:

struct AttachmentLoader {
	const char* error1;
	const char* error2;

const void* const vtable;
#ifdef __cplusplus
	AttachmentLoader() : error1( 0 ), error2( 0 ), vtable( 0 ) { }
#endif
};

That's the only one that I think a fair number of C++ engines would want to overload. Granted, folks can choose to have their own struct with the AttachmentLoader base struct as a member named super (and use malloc) but I found that by adding those one line constructors with initializer lists, I got the best of both worlds. Minimally invasive, and I can derive cleanly and alloc with my own placement new(). So a win-win.

It would be great if those 3 lines could be added to the main Spine-C run-time for at least the AttachmentLoader (and maybe for a few of the other structs too, though they're not as likely to be inherited from). But if I have to re-paste those 3 lines each time I merge the code, well that's not so bad 🙂

I think the constructors are easy to maintain, since they are inlined with the source, so if you add another pointer to const data, like the vtable, then you could add it to the initializer list right there and then. But that's Nate's call.

Big kudos also to Nate for removing some of the wonky old-school C OO stuff that was in there 🙂 I appreciate it

everything looks cleaner now, and you reduced much of the reason I was overriding the classes. It is a better infrastructure now

you've hidden away more of the internals that we don't really need to mess with. So that's a win.

And thanks for adding the memory allocation scheme. That works for me (though I haven't used it yet).

All in all, I've got it integrating quite nicely into my engine, extending it cleanly, and everything is going good. I am having one issue with switching skins, I think there's a bug in Skin_setSkin(), but I'll cover that in another post.

Thanks to both of you for the help and work,
Ken

Nate

@djr, merged your PR, thanks!! 🙂

@Tiggertooth, I had wondered where you went. 😉 The ctor looks reasonable, committed. AFAIK, AttachmentLoader is the only "class" that needs extension. It could even be a function pointer, but not sure it is worth changing.

For setSkin, you need to call setSlotsToBindPose since it doesn't know if you want to change the attachments when you set the skin. It tries to replace attachments from the old skin if they were attached and exist in the new skin, but when setting the skin for the first time there is no old skin.

Tiggertooth

Thanks! I agree that AttachmentLoader is really the only candidate in nearly all cases. (I put in some additional things to allow for some in-engine analysis of the skeleton and its raw data through my own UI system, and tracking things like which bones were selected and things like that, performance, memory usage, etc. But that's a really edge case).

I was not calling setSlotsToBindPose(), so that's most certainly the root of my issue. Let me incorporate that.

Nate

After the recent refactoring, most of the objects are just data to be consumed by something else. Did you need to modify anything to display you analysis?

Yeah, setSlotsToBindPose after setSkin is a bit of a gotcha, but I don't want setSkin to modify the attachments when someone might not want it to.

Chounard

If you extend AttachmentLoader (rather than RegionAttachmentLoader), how do you deal with the typecast near the bottom of SkeletonJson_readSkeletonData?

Nate

I guess you mean Attachment rather than RegionAttachment? You shouldn't have to extend RegionAttachment, it just holds data read from the JSON. It also holds "void* texture" which the AttachmentLoader can set.

Chounard

Yes, that's absolutely what I meant. Clearly I need to sleep more. 😛

Thanks, Nate.