Plan for source file cleanup: Difference between revisions

From OHRRPGCE-Wiki
Jump to navigation Jump to search
(long-term cleanup overview)
 
(→‎Shared source files: half of this was obsolete, deleted)
 
(6 intermediate revisions by 3 users not shown)
Line 1: Line 1:
The [[OHRRPGCE]] [[Source|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.
The [[OHRRPGCE]] [[Source|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.


Sometime after [[ubersetzung]], we should clean up the code origanization. This will make particular subs and functions easier to find, and it will make it easier to identify redundant code. This cleanup can happen incrementally in small steps. it does not have to happen all at once (and in fact it is better if it doesn't happen all at once)
See [[Guide to source files]] for more information.


==[[GAME]] source files==
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)
game.bas        Main module
bmod.bas        Battle module
bmodsubs.bas    Mostly battle code
menustuf.bas    Unsorted code
moresubs.bas    Unsorted code
yetmore.bas      Unsorted code
yetmore2.bas    Unsorted code


# Make a new source file such as gamesubs.bas
Note: this article is badly out of date
# 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
# When we identify clear logical divisions of code, create new modules named appropriately for them, and move code out of gamesubs.bas


==[[CUSTOM]] source files==
=-lang fb cleanup (DONE)=
custom.bas      Main module
drawing.bas      Unsorted code, including graphics drawing
flexmenu.bas    Unsorted code, including menu handling
mapsubs.bas      Unsorted code, including map editor
menus.bas        Unsorted code
subs.bas        Unsorted code
subs2.bas        Unsorted code
subs3.bas        Unsorted code


# Make new source file such as customsubs.bas
All code should be cleaned up to compile with -lang fb.
# move code from menus, subs, subs2, and subs2 into customsubs.bas and eliminate those files when empty
# Move non-graphics drawing code out of drawing.bas
# Move non-map-editor code out of mapsubs.bas
# Move non menu handling related code out of flexmenu
## Evaluate whether (after some cleanup and rewriting) flexmenu is worth sharing between game and custom
# When we identify clear logical divisions of code, create new modules named appropriately for them, and move code out of customsubs.bas


==Shared source files==
Full details of the fb compiler dialect are here: http://www.freebasic.net/wiki/wikka.php?wakka=CompilerDialects
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


allmodex.bas    shared game library
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.
gfx_*.bas        graphics backends
music_*.bas      music backends
bam2mid.bas      bam music converter
common.bas      shared code for both game and custom
compat.bas      backwards compatability code
loading.bas      shared code for RPG lump handling
util.bas        shared code that ''should'' not depend on any other code


* Fix util.bat's dependence on compat.bat (which leads to dependence on other stuff too)
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.
* Evaluate whether or not compat.bat can be completely removed now that we have abandoned QB45
 
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 (DONE)=
 
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=
 
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==
* 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==
* 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==
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=
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]]
 
* split misc.bas apart to util.bas and common.bas
* Find other code that can be shared between game and custom and move it into common.bas
* 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 game and custom do their own special-case handling of RPG lumps, and replace them with calls to loading.bas
<s>
* 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
* 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.
</s>


=See Also=
=See Also=
* [[Source#Development_Plans|Development Plans]]
* [[Plans|Development Plans]]
 
[[Category:OHRRPGCE Development]]

Latest revision as of 06:06, 6 April 2017

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)

Note: this article is badly out of date

-lang fb cleanup (DONE)[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 (DONE)[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

  • split misc.bas apart to util.bas and common.bas
  • 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.

See Also[edit]