Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions MapChooserSharp/Modules/MapCycle/McsMapCycleController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,13 @@ private void OnClientPutInServer(int slot)
private void OnMapStart(string mapName)
{
ExtendCount = 0;

DecrementAllMapCooldown(CurrentMap);

CurrentMap = NextMap;
NextMap = null;

DecrementAllMapCooldown(CurrentMap);
ApplyCooldownToCurrentMap(CurrentMap);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There's a logical error in the reordered operations. The cooldown is now being applied to the map that is starting (NextMap), not the map that just finished (the old CurrentMap).

With this change, CurrentMap is updated to NextMap before the cooldown logic is executed. This means ApplyCooldownToCurrentMap is called with the new map, putting it on cooldown immediately, while the map that just ended doesn't get its cooldown applied.

To fix this, the cooldown logic should be executed before CurrentMap is updated. Note that ApplyCooldownToCurrentMap should be made asynchronous as suggested in a separate comment, so you will need to await it and mark OnMapStart as async void.

        DecrementAllMapCooldown(CurrentMap);
        await ApplyCooldownToCurrentMap(CurrentMap);
        
        CurrentMap = NextMap;
        NextMap = null;


Server.NextFrame(ExecuteMapLimitConfig);

// Wait for first people joined
Expand Down Expand Up @@ -323,22 +324,24 @@ private void DecrementAllMapCooldown(IMapConfig? previousMap)
if (value.MapCooldown.CurrentCooldown > 0)
value.MapCooldown.CurrentCooldown--;
}

// Set previous map cooldown if defined in config
if (previousMap != null)
{
_mcsDatabaseProvider.MapInfoRepository.UpsertMapCooldownAsync(previousMap.MapName, previousMap.MapCooldown.MapConfigCooldown).ConfigureAwait(false);
previousMap.MapCooldown.CurrentCooldown = previousMap.MapCooldown.MapConfigCooldown;
}

private void ApplyCooldownToCurrentMap(IMapConfig? currentMap)
{
if (currentMap == null)
return;

foreach (IMapGroupSettings setting in previousMap.GroupSettings)
{
_mcsDatabaseProvider.GroupInfoRepository.UpsertGroupCooldownAsync(setting.GroupName, setting.GroupCooldown.MapConfigCooldown).ConfigureAwait(false);
setting.GroupCooldown.CurrentCooldown = setting.GroupCooldown.MapConfigCooldown;
}
_mcsDatabaseProvider.MapInfoRepository.UpsertMapCooldownAsync(currentMap.MapName, currentMap.MapCooldown.MapConfigCooldown).ConfigureAwait(false);
currentMap.MapCooldown.CurrentCooldown = currentMap.MapCooldown.MapConfigCooldown;

foreach (IMapGroupSettings setting in currentMap.GroupSettings)
{
_mcsDatabaseProvider.GroupInfoRepository.UpsertGroupCooldownAsync(setting.GroupName, setting.GroupCooldown.MapConfigCooldown).ConfigureAwait(false);
setting.GroupCooldown.CurrentCooldown = setting.GroupCooldown.MapConfigCooldown;
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This method performs asynchronous database operations but doesn't await them. This 'fire-and-forget' approach can lead to unhandled exceptions that could crash the server if the database operations fail. To make this more robust, the method should be async Task and await the database calls.

Additionally, for better clarity, consider renaming this method to ApplyCooldownToMap and its parameter to map. The current name ApplyCooldownToCurrentMap is misleading because the cooldown should be applied to the map that has just finished, not necessarily the one that is 'current' when the method is called.

    private async Task ApplyCooldownToMap(IMapConfig? map)
    {
        if (map == null)
            return;
            
        await _mcsDatabaseProvider.MapInfoRepository.UpsertMapCooldownAsync(map.MapName, map.MapCooldown.MapConfigCooldown).ConfigureAwait(false);
        map.MapCooldown.CurrentCooldown = map.MapCooldown.MapConfigCooldown;
        
        foreach (IMapGroupSettings setting in map.GroupSettings)
        {
            await _mcsDatabaseProvider.GroupInfoRepository.UpsertGroupCooldownAsync(setting.GroupName, setting.GroupCooldown.MapConfigCooldown).ConfigureAwait(false);
            setting.GroupCooldown.CurrentCooldown = setting.GroupCooldown.MapConfigCooldown;
        }
    }




private HookResult OnRoundEnd(EventRoundEnd @event, GameEventInfo info)
{
if (!_isMapStarted)
Expand Down
Loading