GameRefactoring - 123jimin/unnamed-sdvx-clone GitHub Wiki

The refactoring is currently being done under the 202204-game-refactor branch.

The scope of the refactoring is ones related to Game and Game_Impl class, not the entire game.

Current Game.cpp is over 3500-LoC monolith code, including logics for Lua bidings, particle system, rendering, practice mode, multiplayer, and replays. LoC itself is not a significant problem here, but multiple hundreds-of-lines of functions with various purposes put in a large PImpl class makes the code confusing to navigate - at least for me who have seen the code for several years.

I have many future suggestions for features in USC, and certainly other devs have ambitious plans on USC too.

• HitStat rendering on each note
• In-game chart editing and live reloading
• Introducing mouse-navigatable UI for the practice mode
• More sophiscated calibration mode, including microphone-based calibrations
• Make charts keep playing after audio has ended
• Make audio keep playing after charts have ended

However, I can picture that while adding these to current codebase is definitely feasible, it will be likely done by adding sprinkles of if-elses inside random functions of Game_Impl, and it will only make Game.cpp more complex and unreadable.

There have been discussions on refactoring/rewriting the codebase, such as this one in '18 or this one in '19, but they seem to be inactice now. I suspect that the reason behind these discussion being stuck is a lack of benefit vs. cost:

• While replacing raw pointers and Ref to STL smart pointers is certainly useful, itself is little more useful than removing "a few crashes"
  • There are some lifetime-related crash issues but they don't impact a lot of players now.
• Rewriting the code including implementing various specific features (multiplayer, practice mode, replay, calibration, ...) is quite tedious, and will cause a lot of regressions.

I propose to refactor Game.cpp, with a method which can be done incrementally, and would result in cleaner and more managable code.

• Replace Game and Game_Impl with Gameplay/GameplayScene
  • PImpl pattern will not be used.
    • I think that doing so will make the codebase more navigatable using the header file.
    • Instead, I believe that component-based structure will remove much of the implementation details in the header file.
  • I chose -Scene instead of current convention -Screen, since -Screen gives me impression that it's a GUI-related class.
  • Put .hpp header files alongside .cpp files, not under /include.
• GameplayScene consists of many components with different, mostly orthogonal functions. (Held by std::unique_ptr)
  • Optimally communication between different components are only done in GameplayScene, but there might be some exceptions.
  • Even in those exceptions, explictly specifying dependent components as const Component& function args is preferred.
    • e.g. defining GameplayScene& m_gameplay field for a component is frowned upon. It will encourage devs to write mangled code.
    • Also, I specifically want for PlaybackComponent to work independent of other components (as much as possible).
  • Delegates will be extensively used to send 'signals' between components. (Any opinion on this?)
  • PlaybackComponent: Codes related to playback of the chart
    • Consider it as a <video/> for a chart.
    • Consists of BeatmapPlayback, AudioPlayback, Camera, Track, particle system, ...
    • Example functions: Render(...), SetStart(...), Pause(), Play(), GetCurrMapTime(), GetCurrMapObjects(), ...
    • It must not manage player inputs / autoplay status / ... directly, but instead be fed by function arguments.
  • InputComponent: Codes related to input handling
    • It might not be necessary (putting those logics in GameplayScene is fine by me)
  • ScoringComponent: Codes related to gauge, score, and judgement management
    • Basically the old Scoring
  • FGBGComponent: for rendering foreground/background
  • LuaComponent
  • DebugHUDComponent
  • MultiplayerComponent, ReplayComponent, PracticeComponent, ...
• Make new and refactored C++ code modern
  • Use std::array, std::vector (or Vector?) for lists
  • Use std::unique_ptr, std::optional, and references instead of raw pointers and bool flags.
    • Don't think that a shared pointer is necessary (unless old code requires using it)
  • Custom destructors must be as minimal as possible. Reduce usage of new and delete as much as possible.

• Implementing new features (it should be done separatly)
• Refactoring with future plans in mind
  • Premature refactoring almost always goes wrong.
  • Refactor further during implementing new features.
  • This refactoring will make those further refactorings easier.
• Refactoring codes not directly related to Game
• Renaming other -Screen classes into -Scene for now

Each item will be a separate PR.

1. Move playback-related codes into PlaybackComponent, and move rest of the codes into GameplayScene.
   • It's currently being done on the 202204-game-refactor branch.
   • May rename Scoring to ScoringComponent.
   • First PR will not separate other parts of Game into components.
     • Current multiplayer, replay, practice-mode, and Lua codes will still reside under GameplayScene.
     • If separating PlaybackComponent alone will be PR-worthy, then it will prove that further refactoring is beneficial...
2. Separate Lua-related codes into LuaComponent, and the debug HUD into DebugHUDComponent.
3. Separate other components.

Separating each component will be done in a simple manner, which will reduce regressions and also can be tested progressively.

1. Choose a field Field m_field in Game_Impl to move under the component FooComponent.
2. Create a temporary getter Field& FooComponent::TEMP_field() { return m_field; }.
3. Replace all occurences of m_field in Game_Impl into m_fooComponent->TEMP_field().
4. Repeat 1-3 until a logical piece of code is found which only deals with m_fooComponent. Move that piece of code into FooComponent.
5. If a logical piece of code mainly deals with m_fooComponent, but it requires fields from some other external codes, then create a function that receives temporary arguments prefixed TEMP_. Deal with those arguments later.
6. After all relevant fields are moved, improve the public interface of FooComponent such that all TEMP_ usages can be removed.

• m_hispeed
• cMod
• cModSpeed
• m_speedMod
• m_modSpeed
• m_hideLane
• m_beatmap
• old m_playback (m_beatmapPlayback)
• m_audioPlayback
• combined audio offset
• m_track
• m_lastMapTime
• m_endTime
• m_camera
• m_currentTiming
• m_fxVolume
• m_slamVolume
• m_slamSample
• m_clickSamples
• m_fxSamples
• m_rollIntensity
• m_manualTiltEnabled
• particleMaterial
• basicParticleTexture
• m_particleSystem
• m_laserFollowEmitters
• m_holdEmitters
• m_showCover
• m_hiddenObjects
• m_permanentlyHiddenObjects
• loader for async-loading Track

Game.cpp still looks kinda messy and the API needs to be improved.

• Normal gameplay
• Autoplay
  • Normal play
  • Pause
  • Skip forward/backwards
• Speed changes
  • XMod
  • MMod
  • CMod
  • Mod changes mid play
• Multiplayer
• Practice mode
  • Chart scrolling and navigation
  • Play once
  • Play with missions
  • Play with "max # of measures to rewind" option
  • Play with speed-increase modifiers
• Challenge
• Replay
• Debug HUD