3991: Implement upstream download code style r=heinrich5991 a=ChillerDragon

In https://github.com/teeworlds/teeworlds/pull/2556 teeworlds introduced temporary download files for maps. DDNet already has that but a bit different. Those temporary files are now also deleted on error like it is done in the upstream. The main goal of this pr is to reduce code diff to upstream.

Tested official ddnet maps over http and other maps.

## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robert.mueller@student.uni-siegen.de>
Co-authored-by: ChillerDragon <ChillerDragon@gmail.com>
This commit is contained in:
bors[bot] 2021-09-13 12:00:29 +00:00 committed by GitHub
commit 2916f1b7d4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 76 additions and 76 deletions

View file

@ -317,9 +317,10 @@ CClient::CClient() :
// map download
m_aMapdownloadFilename[0] = 0;
m_aMapdownloadFilenameTemp[0] = 0;
m_aMapdownloadName[0] = 0;
m_pMapdownloadTask = NULL;
m_MapdownloadFile = 0;
m_MapdownloadFileTemp = 0;
m_MapdownloadChunk = 0;
m_MapdownloadSha256Present = false;
m_MapdownloadSha256 = SHA256_ZEROED;
@ -460,9 +461,12 @@ void CClient::SendReady()
void CClient::SendMapRequest()
{
if(m_MapdownloadFile)
io_close(m_MapdownloadFile);
m_MapdownloadFile = Storage()->OpenFile(m_aMapdownloadFilename, IOFLAG_WRITE, IStorage::TYPE_SAVE);
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
m_MapdownloadFileTemp = Storage()->OpenFile(m_aMapdownloadFilenameTemp, IOFLAG_WRITE, IStorage::TYPE_SAVE);
CMsgPacker Msg(NETMSG_REQUEST_MAP_DATA, true);
Msg.AddInt(m_MapdownloadChunk);
SendMsg(&Msg, MSGFLAG_VITAL | MSGFLAG_FLUSH);
@ -804,9 +808,12 @@ void CClient::DisconnectWithReason(const char *pReason)
m_MapdownloadChunk = 0;
if(m_pMapdownloadTask)
m_pMapdownloadTask->Abort();
if(m_MapdownloadFile)
io_close(m_MapdownloadFile);
m_MapdownloadFile = 0;
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
m_MapdownloadFileTemp = 0;
m_MapdownloadSha256Present = false;
m_MapdownloadSha256 = SHA256_ZEROED;
m_MapdownloadCrc = 0;
@ -1219,24 +1226,43 @@ const char *CClient::LoadMap(const char *pName, const char *pFilename, SHA256_DI
return 0;
}
static void FormatMapDownloadFilename(const char *pName, const SHA256_DIGEST *pSha256, int Crc, bool Temp, char *pBuffer, int BufferSize)
{
char aSuffix[32];
if(Temp)
{
str_format(aSuffix, sizeof(aSuffix), ".%d.tmp", pid());
}
else
{
str_copy(aSuffix, ".map", sizeof(aSuffix));
}
if(pSha256)
{
char aSha256[SHA256_MAXSTRSIZE];
sha256_str(*pSha256, aSha256, sizeof(aSha256));
str_format(pBuffer, BufferSize, "downloadedmaps/%s_%s%s", pName, aSha256, aSuffix);
}
else
{
str_format(pBuffer, BufferSize, "downloadedmaps/%s_%08x%s", pName, Crc, aSuffix);
}
}
const char *CClient::LoadMapSearch(const char *pMapName, SHA256_DIGEST *pWantedSha256, int WantedCrc)
{
const char *pError = 0;
char aBuf[512];
char aWanted[256];
char aWantedSha256[SHA256_MAXSTRSIZE];
char aWanted[SHA256_MAXSTRSIZE + 16];
aWanted[0] = 0;
if(pWantedSha256)
{
char aWantedSha256[SHA256_MAXSTRSIZE];
sha256_str(*pWantedSha256, aWantedSha256, sizeof(aWantedSha256));
str_format(aWanted, sizeof(aWanted), "sha256=%s", aWantedSha256);
str_format(aWanted, sizeof(aWanted), "sha256=%s ", aWantedSha256);
}
else
{
str_format(aWanted, sizeof(aWanted), "crc=%08x", WantedCrc);
}
str_format(aBuf, sizeof(aBuf), "loading map, map=%s wanted %s", pMapName, aWanted);
str_format(aBuf, sizeof(aBuf), "loading map, map=%s wanted %scrc=%08x", pMapName, aWanted, WantedCrc);
m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "client", aBuf);
SetState(IClient::STATE_LOADING);
@ -1247,20 +1273,19 @@ const char *CClient::LoadMapSearch(const char *pMapName, SHA256_DIGEST *pWantedS
return pError;
// try the downloaded maps
FormatMapDownloadFilename(pMapName, pWantedSha256, WantedCrc, false, aBuf, sizeof(aBuf));
pError = LoadMap(pMapName, aBuf, pWantedSha256, WantedCrc);
if(!pError)
return pError;
// backward compatibility with old names
if(pWantedSha256)
{
str_format(aBuf, sizeof(aBuf), "downloadedmaps/%s_%s.map", pMapName, aWantedSha256);
}
else
{
str_format(aBuf, sizeof(aBuf), "downloadedmaps/%s_%08x.map", pMapName, WantedCrc);
FormatMapDownloadFilename(pMapName, 0, WantedCrc, false, aBuf, sizeof(aBuf));
pError = LoadMap(pMapName, aBuf, pWantedSha256, WantedCrc);
if(!pError)
return pError;
}
pError = LoadMap(pMapName, aBuf, pWantedSha256, WantedCrc);
if(!pError)
return pError;
// search for the map within subfolders
char aFilename[IO_MAX_PATH_LENGTH];
@ -1514,28 +1539,6 @@ bool CClient::ShouldSendChatTimeoutCodeHeuristic()
return IsDDNet(&m_CurrentServerInfo);
}
static void FormatMapDownloadFilename(const char *pName, SHA256_DIGEST *pSha256, int Crc, bool Temp, char *pBuffer, int BufferSize)
{
char aHash[SHA256_MAXSTRSIZE];
if(pSha256)
{
sha256_str(*pSha256, aHash, sizeof(aHash));
}
else
{
str_format(aHash, sizeof(aHash), "%08x", Crc);
}
if(Temp)
{
str_format(pBuffer, BufferSize, "%s_%s.map.%d.tmp", pName, aHash, pid());
}
else
{
str_format(pBuffer, BufferSize, "%s_%s.map", pName, aHash);
}
}
static CServerCapabilities GetServerCapabilities(int Version, int Flags)
{
CServerCapabilities Result;
@ -1666,16 +1669,18 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket)
}
else
{
char aFilename[IO_MAX_PATH_LENGTH];
FormatMapDownloadFilename(pMap, pMapSha256, MapCrc, false, aFilename, sizeof(aFilename));
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFileTemp);
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
char aTempFilename[IO_MAX_PATH_LENGTH];
FormatMapDownloadFilename(pMap, pMapSha256, MapCrc, true, aTempFilename, sizeof(aTempFilename));
str_format(m_aMapdownloadFilename, sizeof(m_aMapdownloadFilename), "downloadedmaps/%s", aTempFilename);
// start map download
FormatMapDownloadFilename(pMap, pMapSha256, MapCrc, false, m_aMapdownloadFilename, sizeof(m_aMapdownloadFilename));
FormatMapDownloadFilename(pMap, pMapSha256, MapCrc, true, m_aMapdownloadFilenameTemp, sizeof(m_aMapdownloadFilenameTemp));
char aBuf[256];
str_format(aBuf, sizeof(aBuf), "starting to download map to '%s'", m_aMapdownloadFilename);
str_format(aBuf, sizeof(aBuf), "starting to download map to '%s'", m_aMapdownloadFilenameTemp);
m_pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "client/network", aBuf);
m_MapdownloadChunk = 0;
@ -1693,11 +1698,11 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket)
{
char aUrl[256];
char aEscaped[256];
EscapeUrl(aEscaped, sizeof(aEscaped), aFilename);
EscapeUrl(aEscaped, sizeof(aEscaped), m_aMapdownloadFilename + 15); // cut off downloadedmaps/
bool UseConfigUrl = str_comp(g_Config.m_ClMapDownloadUrl, "https://maps2.ddnet.tw") != 0 || m_aMapDownloadUrl[0] == '\0';
str_format(aUrl, sizeof(aUrl), "%s/%s", UseConfigUrl ? g_Config.m_ClMapDownloadUrl : m_aMapDownloadUrl, aEscaped);
m_pMapdownloadTask = std::make_shared<CGetFile>(Storage(), aUrl, m_aMapdownloadFilename, IStorage::TYPE_SAVE, CTimeout{g_Config.m_ClMapDownloadConnectTimeoutMs, g_Config.m_ClMapDownloadLowSpeedLimit, g_Config.m_ClMapDownloadLowSpeedTime});
m_pMapdownloadTask = std::make_shared<CGetFile>(Storage(), aUrl, m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE, CTimeout{g_Config.m_ClMapDownloadConnectTimeoutMs, g_Config.m_ClMapDownloadLowSpeedLimit, g_Config.m_ClMapDownloadLowSpeedTime});
Engine()->AddJob(m_pMapdownloadTask);
}
else
@ -1714,19 +1719,19 @@ void CClient::ProcessServerPacket(CNetChunk *pPacket)
const unsigned char *pData = Unpacker.GetRaw(Size);
// check for errors
if(Unpacker.Error() || Size <= 0 || MapCRC != m_MapdownloadCrc || Chunk != m_MapdownloadChunk || !m_MapdownloadFile)
if(Unpacker.Error() || Size <= 0 || MapCRC != m_MapdownloadCrc || Chunk != m_MapdownloadChunk || !m_MapdownloadFileTemp)
return;
io_write(m_MapdownloadFile, pData, Size);
io_write(m_MapdownloadFileTemp, pData, Size);
m_MapdownloadAmount += Size;
if(Last)
{
if(m_MapdownloadFile)
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFile);
m_MapdownloadFile = 0;
io_close(m_MapdownloadFileTemp);
m_MapdownloadFileTemp = 0;
}
FinishMapDownload();
}
@ -2337,7 +2342,7 @@ void CClient::ResetMapDownload()
m_pMapdownloadTask->Abort();
m_pMapdownloadTask = NULL;
}
m_MapdownloadFile = 0;
m_MapdownloadFileTemp = 0;
m_MapdownloadAmount = 0;
}
@ -2350,18 +2355,11 @@ void CClient::FinishMapDownload()
m_MapdownloadTotalsize = -1;
SHA256_DIGEST *pSha256 = m_MapdownloadSha256Present ? &m_MapdownloadSha256 : 0;
char aTmp[IO_MAX_PATH_LENGTH];
char aMapFileTemp[IO_MAX_PATH_LENGTH];
char aMapFile[IO_MAX_PATH_LENGTH];
FormatMapDownloadFilename(m_aMapdownloadName, pSha256, m_MapdownloadCrc, true, aTmp, sizeof(aTmp));
str_format(aMapFileTemp, sizeof(aMapFileTemp), "downloadedmaps/%s", aTmp);
FormatMapDownloadFilename(m_aMapdownloadName, pSha256, m_MapdownloadCrc, false, aTmp, sizeof(aTmp));
str_format(aMapFile, sizeof(aMapFileTemp), "downloadedmaps/%s", aTmp);
Storage()->RenameFile(aMapFileTemp, aMapFile, IStorage::TYPE_SAVE);
Storage()->RemoveFile(m_aMapdownloadFilename, IStorage::TYPE_SAVE);
Storage()->RenameFile(m_aMapdownloadFilenameTemp, m_aMapdownloadFilename, IStorage::TYPE_SAVE);
// load map
pError = LoadMap(m_aMapdownloadName, aMapFile, pSha256, m_MapdownloadCrc);
pError = LoadMap(m_aMapdownloadName, m_aMapdownloadFilename, pSha256, m_MapdownloadCrc);
if(!pError)
{
ResetMapDownload();
@ -2376,10 +2374,11 @@ void CClient::FinishMapDownload()
}
else
{
if(m_MapdownloadFile)
if(m_MapdownloadFileTemp)
{
io_close(m_MapdownloadFile);
m_MapdownloadFile = 0;
io_close(m_MapdownloadFileTemp);
m_MapdownloadFileTemp = 0;
Storage()->RemoveFile(m_aMapdownloadFilenameTemp, IStorage::TYPE_SAVE);
}
ResetMapDownload();
DisconnectWithReason(pError);

View file

@ -181,8 +181,9 @@ class CClient : public IClient, public CDemoPlayer::IListener
// map download
std::shared_ptr<CGetFile> m_pMapdownloadTask;
char m_aMapdownloadFilename[256];
char m_aMapdownloadFilenameTemp[256];
char m_aMapdownloadName[256];
IOHANDLE m_MapdownloadFile;
IOHANDLE m_MapdownloadFileTemp;
int m_MapdownloadChunk;
int m_MapdownloadCrc;
int m_MapdownloadAmount;