Starworld/CODE_CLEANUP_PLAN.md

Code Cleanup Summary

✅ Completed Cleanup (All changes successfully compiled)

1. Standardized Anonymous Namespaces ✅

Before: Inconsistent mix of static functions and anonymous namespaces
After: All internal implementation details use anonymous namespaces

StardustBridge.cpp

• ✅ Converted static candidateSocketPaths() → anonymous namespace
• ✅ Removed StardustBridge::defaultSocketPath() (was public static, only used internally)
• ✅ Updated header comment to reflect socket auto-discovery
• ✅ Alphabetized includes, grouped by category (STL, system headers)

NLPacketCodec.cpp

• ✅ Converted 5 static constexpr bit masks → anonymous namespace constants
• ✅ Converted 4 static helper functions → anonymous namespace:
  • readFileToString()
  • parseEnumValues()
  • parsePacketTypeCount()
  • ensureVersionTable()

DomainDiscovery.cpp

• ✅ Kept existing anonymous namespace for httpGet() and write_cb
• ✅ Converted 2 static JSON helpers → anonymous namespace:
  • findAllStrings()
  • findAllInts()

2. Cleaned Up Includes ✅

Before: Headers duplicated in .cpp files, unsorted includes
After: Alphabetized includes, removed redundancies, grouped by type

StardustBridge.cpp

// Before: 19 unsorted includes
#include <chrono>
#include <sys/socket.h>
#include <vector>
// ... mixed order

// After: Grouped and alphabetized
#include <algorithm>
#include <chrono>
// ... STL headers alphabetically

#include <dlfcn.h>
#include <fcntl.h>
// ... system headers alphabetically

DomainDiscovery.cpp

// Before: Mixed includes with comments
#include <iostream>
#include <string>  // Already in header (std::string)
#include <vector>  // Already in header
// Minimal libcurl-based GET
#include <curl/curl.h>

// After: Clean, alphabetized, no redundant std::string/vector
#include <cstdlib>
#include <cstring>
#include <iostream>
#include <optional>
#include <sstream>

#include <curl/curl.h>
#include <arpa/inet.h>
// ... system headers alphabetically

ModelCache.cpp

// Before: Includes with inline comments
#include <iostream>
#include <thread>
#include <cstring>
// For HTTP downloads - using libcurl (cross-platform)
#include <curl/curl.h>
// For hashing URLs to filenames
#include <openssl/sha.h>

// After: Clean and alphabetized
#include <cstring>
#include <fstream>
#include <iomanip>
#include <iostream>
#include <sstream>
#include <thread>

#include <curl/curl.h>
#include <openssl/sha.h>

3. Removed Public API Bloat ✅

StardustBridge.hpp

Removed: static std::string defaultSocketPath()

• Was a public static method but only used internally in connect()
• Implementation now private in anonymous namespace within .cpp
• Cleaner public API surface

Before (StardustBridge.hpp):

public:
    bool connect(const std::string& socketPath = {});
    // ... other methods
    static std::string defaultSocketPath();  // ❌ Exposed unnecessarily

After (StardustBridge.hpp):

public:
    bool connect(const std::string& socketPath = {});
    // ... other methods
    // defaultSocketPath removed - internal detail only

4. Improved Code Organization ✅

All files now follow consistent pattern:

#include "Header.hpp"

// STL includes (alphabetically)
#include <algorithm>
#include <string>
#include <vector>

// System includes (alphabetically)
#include <curl/curl.h>
#include <sys/socket.h>

namespace {
// Internal constants
constexpr uint32_t MASK = 0x80000000;

// Internal helper functions
void helperFunction() { ... }

} // anonymous namespace

// Public API implementations
void PublicClass::publicMethod() { ... }

Impact Summary

Code Metrics

• Lines removed: ~35 (redundant includes, removed public method)
• Public API surface: Reduced by 1 method (defaultSocketPath)
• Consistency: 100% of internal helpers now use anonymous namespaces
• Build time: No change (same compilation units)
• Binary size: No change (same code, different organization)

Benefits

✅ Clarity: Clear separation of public API vs internal implementation
✅ Consistency: All internal helpers use same pattern (anonymous namespace)
✅ Maintainability: Alphabetized includes easier to scan
✅ Encapsulation: Reduced public API surface
✅ Modern C++: Following best practices (anonymous namespace > static)

Files Modified

1. ✅ src/StardustBridge.cpp - Anonymous namespace, alphabetized includes
2. ✅ src/StardustBridge.hpp - Removed defaultSocketPath()
3. ✅ src/NLPacketCodec.cpp - Anonymous namespace for all helpers
4. ✅ src/DomainDiscovery.cpp - Anonymous namespace, cleaned includes
5. ✅ src/ModelCache.cpp - Alphabetized includes

Verified

✅ Build successful: starworld (305KB), starworld-tests (106KB)
✅ No behavior changes - pure refactoring
✅ No new warnings or errors

What We Did NOT Change (Intentionally Kept)

✅ Good Patterns Already Present

• Anonymous namespaces in ModelCache.cpp: Already well-organized ✅
• Anonymous namespace in OverteClient.cpp: Already well-organized ✅
• Static class SceneSync: Appropriate use of static methods ✅
• Header guards using #pragma once: Modern and clean ✅
• Smart pointers and std::optional: Modern C++ patterns ✅

📝 Functions Kept Public (For Good Reasons)

• parseDomainsFromJson() in DomainDiscovery.hpp: Marked "for tests"
  • Decision: Keep public for future test usage
  • Alternative considered: Move to anonymous namespace
  • Rationale: Exported for testability

🎯 Design Decisions Validated

• Constants in .cpp files (not headers) ✅ Correct - reduces recompilation
• Forward declarations where appropriate ✅ Reduces include dependencies
• Separate .hpp/.cpp files ✅ Clean interface/implementation separation

Next Steps (Future Work)

Optional Further Cleanup

1. ⏭️ Add unit tests that use parseDomainsFromJson() (currently unused)
2. ⏭️ Consider forward declarations to reduce header includes
3. ⏭️ Document remaining design patterns (e.g., SceneSync static class)

Feature Development (Resume)

Now that codebase is cleaned up, ready to continue:

• ATP protocol support
• Parse all entity types from EntityTypes.h
• Entity property updates
• Entity deletion handling
• Testing with real/local Overte server

Lessons Learned

✅ C++ Best Practices Applied

1. Anonymous namespaces > static: Better for translation unit scope
2. Minimize public API: Only expose what's truly needed
3. Organize includes: Alphabetically, grouped by type
4. Remove redundancy: Don't re-include what's in headers
5. Consistency: Apply patterns uniformly across codebase

🎯 Trade-offs Made

• Verbosity: Anonymous namespace adds 2 lines per file
  • Benefit: Clarity outweighs brevity
• Include sorting: Takes time to reorganize
  • Benefit: Easier to spot duplicates and missing includes
• Public API reduction: Removed potentially "useful" utility
  • Benefit: Users can't depend on internal implementation details