Fix save syncing when there are no offline saves

[tudddorrr]

No description provided.

[claude]

🔍 Code Review: Fix save syncing when there are no offline saves

---

🐛 Potential Bugs or Issues

SavesAPI.cs:72-105

Bare catch block swallowing exceptions

• Line 93: The catch block in SyncOfflineSaves silently swallows all exceptions and returns null
• This makes debugging difficult - network failures, serialization errors, or API errors will be invisible
• Consider logging the exception or at least capturing it for diagnostics:

catch (Exception ex)
{
    Debug.LogWarning($"Failed to sync offline save {offlineSave.name}: {ex.Message}");
    return null;
}

Potential null reference when no successful results

• Lines 100-104: If all sync attempts fail, successfulResults will be empty but code proceeds to call DeleteOfflineSaves with an empty array
• While DeleteOfflineSaves has a guard check (line 134), this creates unnecessary method calls and array allocations
• The method should early-return if no saves were successfully synced

---

⚡ Performance Considerations

SavesAPI.cs:79-97

Inefficient async pattern for save creation

• The code creates parallel tasks for all offline saves, but each uses Call(uri, "POST", ...) which likely hits the same endpoint sequentially due to HTTP connection limits
• Consider adding rate limiting or batching if there could be many offline saves (e.g., 50+ saves)
• Unity's main thread may also be blocked during the await Task.WhenAll(tasks) if these are long-running operations

SavesManager.cs:132-141

Multiple file I/O operations

• The new DeleteOfflineSaves method reads and writes the entire offline saves file
• If called multiple times in quick succession, this could cause performance issues
• However, since it's only called once in SyncOfflineSaves, this is acceptable for the current use case

---

🔒 Security Concerns

None identified - the changes maintain the same authentication and authorization patterns as the existing code.

---

🔄 Backwards Compatibility

SavesManager.cs:132-141

API change is additive

• The new DeleteOfflineSaves(int[]) method is a new addition, not a breaking change
• The existing DeleteOfflineSave(int) method remains unchanged
• Offline save file format appears unchanged

SavesAPI.cs:72-105

Method moved from SavesManager to SavesAPI

• The SyncOfflineSaves method was moved from SavesManager (line 154-169 in old code) to SavesAPI as a private method
• This is technically a breaking change if any external code was calling SavesManager.SyncOfflineSaves()
• However, since it's now private in SavesAPI, this suggests it was always intended to be internal
• Consider verifying that no external callers exist in the codebase or in consumer code

---

💡 Summary

The refactoring successfully moves save syncing logic to a more appropriate location (SavesAPI) and adds batch deletion support. The main concerns are:

1. Critical: Bare catch block hiding errors (SavesAPI.cs:93)
2. Minor: Potential inefficiency with many parallel HTTP requests
3. Minor: Verify backwards compatibility for the method relocation