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)

Contents 1 OPTION EXPLICIT cleanup 2 OPTION BYVAL cleanup 3 GOSUB cleanup 4 File Organization 4.1 Merging 4.2 Splitting 4.3 Subdirectories 5 Shared source files 6 See Also

[edit] OPTION EXPLICIT cleanup

All the old code used to use implicit variable declaration in the QuickBasic style. This is somewhat error prone, and combined with other flaws such as short vague variable names and over-use of global scope and gosub scope, it becomes very error prone.

Some files have already been cleaned up using the OPTION EXPLICIT command. Other files have OPTION EXPLICIT somewhere in the middle of the file, and they can be cleaned up a few subs and functions at a time as it is moved up the file.

[edit] OPTION BYVAL cleanup

QB and FB -lang deprecated pass integer and pointer arguments to SUBs/FUNCTIONs BYREF by default, which means the recieving function can accidentally modify the argument. Usage of BYREF leads to such extremely subtle but very-much-crashing bugs that I can't even understand the BYREF bugs I previously presented anymore.

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 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. Prehaps we could apply OPTION BYVAL on a file-by-file basis, but function declarations will probably have to remain explicit?

[edit] GOSUB cleanup

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 FreeBasic's semi-deprecated support for GOSUB.

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 that represents the shared state of a particular menu (most GOSUB blocks are within editor menus or in-game menus)
• Allocate one of these State 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.

[edit] File Organization

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.

[edit] Merging

• 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

[edit] Splitting

• 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)

[edit] Subdirectories

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.

[edit] Shared source files

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.

[edit] See Also

• Development Plans