Plan for source file cleanup

From OHRRPGCE-Wiki
Jump to: navigation, search

The OHRRPGCE source code is broken up into a bunch of disorganized files. This is because the memory limitations of the old QuickBasic compiler required large modules to be split up. With a few exceptions, the code is grouped randomly with little regard for shared functionality.

See Guide to source files for more information.

There are several source code cleanup projects. They don't have to happen simultaneously (unless there is a case where they would be easier to clean up that way)

-lang fb cleanup[edit]

All code should be cleaned up to compile with -lang fb.

Full details of the fb compiler dialect are here: http://www.freebasic.net/wiki/wikka.php?wakka=CompilerDialects

Mainly for us this means all variables must be DIMed with a specific type. No suffixes like $ can be used. This also means that the default passing convention changes from byref to byval, so we have to take care to explicitly byref things that need it. The -w pedantic option might be helpful for finding variables that don't specify if they are byval or byref, but they still need some manual examination.

Most integers should be passed byval unless there is a good reason why they need to be changeable. However, when in doubt, or confused, passing integers byref will at least match the old behavior.

Strings should never be passed byval; that passes them as zstrings (not ztring ptrs), which is something completely different (and the FB devs very much regret)

UDTs should almost always be passed byref. They should only be passed byval if you want modify a copy for some reason; something which doesn't seem to be done anywhere in the source.

All (?) the code in allmodex.bas and util.bas explicitly declares arguments either byval or byref (it's byval is the vast maority of cases), which is the way around this problem, but frankly ugly and annoying.

Because of our GOSUB/RETRACE hack we do not need to remove all GOSUB to be able to compile with -lang fb

GOSUB cleanup[edit]

Much of the old code uses GOSUB blocks instead of SUBs and FUNCTIONs. This is hell on refactoring, and causes global-variable side effects to pillage and burn the countryside.

All new code should avoid GOSUB and use real SUBs and FUNCTIONs instead. Old GOSUBs need to be cleaned up too, to make the code more maintainable, and to move away from relying on our GOSUB/RETRACE hack.

Many of the remaining GOSUBs depend heavily on sharing variables from their caller's scope. Here is one method of cleaning up such code.

  • Create a data TYPE like SomethingState that represents the shared state of a particular menu (most GOSUB blocks are within editor menus or in-game menus)
  • Allocate one of these SomethingState types for the editor to use.
  • Move variables that need to be shared with GOSUBs into the TYPE definition one or two at a time, testing carefully after each.
  • When all shared variables that the GOSUB needs to access are moved into the type, convert the GOSUB into a SUB that accepts the State as an argument
  • Test throughly again.

This approach works well for big complicated editor menus with many large convoluted GOSUBs. For smaller and simpler GOSUBs, it may be easier to just re-write a work-alike SUB from scratch.

GOSUBS that are only called once, can simply be cut-and-pasted inline into the calling code.

File Organization[edit]

Several of the source files are unsorted collections of code with meaningless names. These can be cleaned up by moving subs and functions out of the files, either splitting or combining files into logical groups.


Merging[edit]

  • Make a new source file such as gamesubs.bas
  • Move code from menustuf, moresubs, yetmore, and yetmore2 into gamesubs.bas, and eliminate those files when empty
  • Move non-battle-specific code from bmodsubs to gamesubs

Splitting[edit]

  • When we identify clear logical divisions of code, create new modules named appropriately for them, and move code out of gamesubs.bas
  • bmod.bas, hsinterpreter.bas, drawing.bas, and mapsubs.bas are all examples of this kind of organization (although some of those files are very messy for other reasons)

Subdirectories[edit]

All of our source files are currently jumbled into the same directory. It might make sense to move them to subdirectories, such as gamesrc customsrc and shared. Doing this would require care to make sure that all of our .bi includes still work as they should.

All of the above file organization ideas are somewhat low-priority because it is relatively easy to just find things using "grep" or other search tools. Dividing code into subdirectories might even be a (very small) step backwards in terms of searchability.

Shared source files[edit]

The shared code is already pretty nicely organized. There is much less cleanup work to be done here than there is in the game-specific and custom-specific code. See Guide to source files#Shared files

  • Fix util.bas's dependence on compat.bas (which leads to dependence on other stuff too)
    • or fix compat.bas's dependence on other stuff?...
  • Evaluate whether or not compat.bas can be completely removed now that we have abandoned QB45
    • compat.bas is still full of code we use, but could it be split apart to util.bas and common.bas?
    • in future we are likely to require new compatibility files for the differences between fbc/fbc -gen gcc/fb2c++ and x86/ARM, but the stuff currently in compat.bas won't be part of it
  • Find other code that can be shared between game and custom and move it into common.bas
    • ...or logically grouped into other shared files
    • lots of loading code that you'd expect in loading.bas is in common.bas
  • Find places where game and custom do their own special-case handling of RPG lumps, and replace them with calls to loading.bas
  • Find places where modules other than allmodex.bas depend directly on gfx_*.bas or music_*.bas and replace them with calls to wrapper functions from allmodex.bas
    • Is this really important? The important part is making sure that code only calls the official backend API's that are implemented by all currently maintained backends, and that nothing directly calls some private backend code, or some code that is only implemented by a single backend.
      • There are in fact only two such places: compat.bas commandline handling (which is OK), and joystick stuff in yetmore.bas. Fixing up yetmore.bas might make changing the graphics API easier.

See Also[edit]