This Trac instance is not used for development anymore!

We migrated our development workflow to git and Gitea.
To test the future redirection, replace trac by ariadne in the page URL.

Opened 18 years ago

Last modified 14 years ago

#140 closed task

Large Scale C++ design — at Version 6

Reported by: Jan Wassenberg Owned by: Jan Wassenberg
Priority: Nice to Have Milestone:
Component: Core engine Keywords:
Cc: Patch:

Description (last modified by Jan Wassenberg)

Cleanups inspired by book of same name (am currently reading it).

I think cleaning up our code to reduce inter-module dependencies is quite important. It promises to reduce compile time, simplify releases of parts ripped out of our codebase (e.g. a limited actor viewer), and overall make the code easier to modify (without having to wonder: "hm, what all does this break?").

First idea: sdl is dragged into 6 of 7 of our static library components - ugh! This would be mostly avoidable by just forward-declaring SDL_Event. Unfortunately it is declared as a union (bad design!!), so that is impossible. Simple solution: wrap it in a struct and forward-declare that.[done]

Use pImpl to hide implementation details of simulation classes. Specifically: boost dependency, which is dragged into network as well.

  • wrap sys detect stuff in struct at least
  • remove globals.h - replace with InputRepeater
  • move enums from header scope into class, e.g. Paint::ORANGE
  • Remove X-Macros. As nifty as they are, unfortunately they are a barrier to static code analysis and VisualAssist.

This means: make apphooks.h and glext_funcs.h regular headers (with standard function decls), adding a note on what all needs to be done to add a new function. Since using those functions is more frequent than adding a new one, this makes sense overall.

However, I am still undecided about doing the same for liberrors.h. Splitting that up into separate header files would avoid a full recompile when adding errors, but would complicate the errcode -> descriptive string mapping and (more importantly) no longer be type safe. The current approach of one big enum containing all error values triggers warnings when someone returns 0 instead of INFO_OK; that would no longer be possible after splitting up into separate headers (because specific modules like VFS with their own error codes would want to use generic ones like TIMED_OUT or OK often) Done - errors are now in ERR namespace and split over multiple headers (thanks, Philip, for the excellent idea!). Type safety is still possible via Lint's "strong typing".

Change History (6)

comment:1 by Jan Wassenberg, 18 years ago

(In [4252]) # sweng forward declare SDL_event. remove SDL dependency of network code. refs #140

comment:2 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:3 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:4 by Jan Wassenberg, 18 years ago

  • wrap sys detect stuff in struct at least
  • remove globals.h - replace with InputRepeater
  • move enums from header scope into class, e.g. Paint::ORANGE
  • Remove X-Macros. As nifty as they are, unfortunately they are a barrier to static code analysis and VisualAssist.

This means: make apphooks.h and glext_funcs.h regular headers (with standard function decls), adding a note on what all needs to be done to add a new function. Since using those functions is more frequent than adding a new one, this makes sense overall. However, I am still undecided about doing the same for liberrors.h. Splitting that up into separate header files would avoid a full recompile when adding errors, but would complicate the errcode -> descriptive string mapping and (more importantly) no longer be type safe. The current approach of one big enum containing all error values triggers warnings when someone returns 0 instead of INFO_OK; that would no longer be possible after splitting up into separate headers (because specific modules like VFS with their own error codes would want to use generic ones like TIMED_OUT or OK often)

comment:5 by Jan Wassenberg, 18 years ago

Description: modified (diff)

comment:6 by Jan Wassenberg, 18 years ago

Description: modified (diff)

Work on error codes complete (forgot to ref this ticket, doh)

Note: See TracTickets for help on using tickets.