Update Racing.cpp #302
Update Racing.cpp #302
Conversation
There was a problem hiding this comment.
This PR has a lot of issues that lead me to believe you didn't write this code yourself:
- The formatting and design is very opinionated and doesn't factor in the style of the code at all. You use const references which are good in practice but aren't used anywhere else. It's strange that the comments were ripped out, too.
- Your helpers are unsound, and one of them isn't even used at all. The build doesn't pass, either. There's no way you tested this code at all.
Sorry, but we can't accept this as it is.
| #include "Racing.hpp" | ||
|
|
||
| #include "db/Database.hpp" | ||
| #include "servers/CNShardServer.hpp" | ||
|
|
||
| #include "NPCManager.hpp" | ||
| #include "PlayerManager.hpp" | ||
| #include "Missions.hpp" |
There was a problem hiding this comment.
Our headers are organized into a hierarchy to avoid things like cyclic inclusion. These spaces exist to indicate that, so please don't remove them.
| @@ -14,35 +12,125 @@ std::map<int32_t, EPInfo> Racing::EPData; | |||
| std::map<CNSocket*, EPRace> Racing::EPRaces; | |||
| std::map<int32_t, std::pair<std::vector<int>, std::vector<int>>> Racing::EPRewards; | |||
|
|
|||
| static void racingStart(CNSocket* sock, CNPacketData* data) { | |||
| auto req = (sP_CL2FE_REQ_EP_RACE_START*)data->buf; | |||
| namespace { | |||
There was a problem hiding this comment.
Don't use a namespace to isolate the helpers, just make them static so they're local to the file. This is the pattern we use in the rest of the codebase.
| resp.iStartTick = 0; // ignored | ||
| resp.iStartTick = 0; | ||
| resp.iLimitTime = EPData[mapNum].maxTime; | ||
|
|
||
| sock->sendPacket(resp, P_FE2CL_REP_EP_RACE_START_SUCC); | ||
| } | ||
|
|
||
| static void racingGetPod(CNSocket* sock, CNPacketData* data) { | ||
| if (EPRaces.find(sock) == EPRaces.end()) | ||
| return; // race not found | ||
| return; | ||
|
|
||
| auto req = (sP_CL2FE_REQ_EP_GET_RING*)data->buf; | ||
|
|
||
| if (EPRaces[sock].collectedRings.count(req->iRingLID)) | ||
| return; // can't collect the same ring twice |
There was a problem hiding this comment.
I think your formatter might have stripped these comments
| bool validateRaceStart(CNSocket* sock, int32_t startEcomID) { | ||
| if (NPCManager::NPCs.find(startEcomID) == NPCManager::NPCs.end()) | ||
| return false; // starting line agent not found | ||
|
|
||
| if (NPCManager::NPCs.find(req->iStartEcomID) == NPCManager::NPCs.end()) | ||
| return; // starting line agent not found | ||
| int mapNum = MAPNUM(NPCManager::NPCs[startEcomID]->instanceID); | ||
| return EPData.find(mapNum) != EPData.end() && EPData[mapNum].EPID != 0; | ||
| } |
There was a problem hiding this comment.
Should validate that a player is not already in a race.
I would also recommend having an output argument for mapnum to avoid having to recalculate it in the packet handlers.
| int mapNum = MAPNUM(NPCManager::NPCs[req->iStartEcomID]->instanceID); | ||
| if (EPData.find(mapNum) == EPData.end() || EPData[mapNum].EPID == 0) | ||
| return; // IZ not found | ||
| bool validateRaceEnd(CNSocket* sock, int32_t endEcomID) { |
There was a problem hiding this comment.
You're not actually using this helper in Racing::racingEnd :P
| postRanking.Time = timeDiff; | ||
| postRanking.Timestamp = getTimestamp(); | ||
| Database::postRaceRanking(postRanking); | ||
| handleRaceRanking(epInfo, plr, podsCollected, score, timeDiff, resp); |
Refactor Racing module: Introduce helper functions for race validation and reward handling