Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong function deprecated warnings with variables (Issue #1435) #3927

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
63 changes: 50 additions & 13 deletions Server/mods/deathmatch/logic/CResourceChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ void CResourceChecker::CheckLuaSourceForIssues(string strLuaSource, const string
{
CHashMap<SString, long> doneWarningMap;
long lLineNumber = 1;

// Check if this is a UTF-8 script
bool bUTF8 = IsUTF8BOM(strLuaSource.c_str(), strLuaSource.length());

Expand Down Expand Up @@ -542,8 +543,8 @@ void CResourceChecker::CheckLuaSourceForIssues(string strLuaSource, const string
if (checkerMode == ECheckerMode::WARNINGS)
{
m_ulDeprecatedWarningCount++;
CLogger::LogPrintf("WARNING: %s/%s [%s] is encoded in ANSI instead of UTF-8. Please convert your file to UTF-8.\n", strResourceName.c_str(),
strFileName.c_str(), bClientScript ? "Client" : "Server");
CLogger::LogPrintf("WARNING: %s/%s [%s] is encoded in ANSI instead of UTF-8. Please convert your file to UTF-8.\n",
strResourceName.c_str(), strFileName.c_str(), bClientScript ? "Client" : "Server");
}
}
}
Expand All @@ -560,6 +561,11 @@ void CResourceChecker::CheckLuaSourceForIssues(string strLuaSource, const string
lNameOffset += lPos; // Make offset absolute from the start of the file
lPos = lNameOffset + lNameLength; // Adjust so the next pass starts from just after this identifier

// Get some context around the identifier (up to 50 chars before and after)
long contextStart = max(0L, lNameOffset - 50);
long contextEnd = min((long)strLuaSource.length(), lNameOffset + lNameLength + 50);
string strContext = strLuaSource.substr(contextStart, contextEnd - contextStart);

string strIdentifierName(strLuaSource.c_str() + lNameOffset, lNameLength);

// In-place upgrade...
Expand All @@ -586,11 +592,23 @@ void CResourceChecker::CheckLuaSourceForIssues(string strLuaSource, const string
if (checkerMode == ECheckerMode::WARNINGS)
{
// Only do the identifier once per file
if (doneWarningMap.find(strIdentifierName) == doneWarningMap.end())
string strContextKey = strIdentifierName + ":" + std::to_string(lLineNumber);
YelehaUwU marked this conversation as resolved.
Show resolved Hide resolved
if (doneWarningMap.find(strContextKey) == doneWarningMap.end())
{
doneWarningMap[strIdentifierName] = 1;
if (!bCompiledScript) // Don't issue deprecated function warnings if the script is compiled, because we can't upgrade it
IssueLuaFunctionNameWarnings(strIdentifierName, strFileName, strResourceName, bClientScript, lLineNumber);
doneWarningMap[strContextKey] = 1;
if (!bCompiledScript) // Don't issue deprecated function warnings if the script is compiled
{
// Skip variable declarations and references
if (strContext.find("local " + strIdentifierName) == string::npos &&
strContext.find("=" + strIdentifierName) == string::npos)
{
YelehaUwU marked this conversation as resolved.
Show resolved Hide resolved
// Only check for function calls
if (strContext.find(strIdentifierName + "(") != string::npos)
{
IssueLuaFunctionNameWarnings(strIdentifierName, strFileName, strResourceName, bClientScript, lLineNumber, strContext);
}
YelehaUwU marked this conversation as resolved.
Show resolved Hide resolved
}
}
CheckVersionRequirements(strIdentifierName, bClientScript);
}
}
Expand Down Expand Up @@ -715,12 +733,20 @@ bool CResourceChecker::UpgradeLuaFunctionName(const string& strFunctionName, boo
//
///////////////////////////////////////////////////////////////
void CResourceChecker::IssueLuaFunctionNameWarnings(const string& strFunctionName, const string& strFileName, const string& strResourceName, bool bClientScript,
unsigned long ulLineNumber)
unsigned long ulLineNumber, const string& strContext)
{
// Skip if this is clearly a variable declaration or usage
if (strContext.find("local " + strFunctionName) != string::npos ||
strContext.find("=" + strFunctionName) != string::npos)
return;

// Only proceed if it looks like a function call
if (strContext.find(strFunctionName + "(") == string::npos)
return;

string strHow;
CMtaVersion strVersion;
ECheckerWhatType what = GetLuaFunctionNameUpgradeInfo(strFunctionName, bClientScript, strHow, strVersion);

if (what == ECheckerWhat::NONE)
return;

Expand All @@ -740,7 +766,6 @@ void CResourceChecker::IssueLuaFunctionNameWarnings(const string& strFunctionNam
strTemp.Format("%s %s because <min_mta_version> %s setting in meta.xml is below %s", strFunctionName.c_str(), strHow.c_str(),
bClientScript ? "Client" : "Server", strVersion.c_str());
}

CLogger::LogPrint(SString("WARNING: %s/%s(Line %lu) [%s] %s\n", strResourceName.c_str(), strFileName.c_str(), ulLineNumber,
bClientScript ? "Client" : "Server", *strTemp));
}
Expand All @@ -755,21 +780,33 @@ void CResourceChecker::IssueLuaFunctionNameWarnings(const string& strFunctionNam
ECheckerWhatType CResourceChecker::GetLuaFunctionNameUpgradeInfo(const string& strFunctionName, bool bClientScript, string& strOutHow,
CMtaVersion& strOutVersion)
{
// Early exit if this is likely a variable assignment
if (strFunctionName.find('=') != string::npos)
YelehaUwU marked this conversation as resolved.
Show resolved Hide resolved
return ECheckerWhat::NONE;

static CHashMap<SString, SDeprecatedItem*> clientUpgradeInfoMap;
static CHashMap<SString, SDeprecatedItem*> serverUpgradeInfoMap;

if (clientUpgradeInfoMap.size() == 0)
{
// Make maps to speed things up
for (uint i = 0; i < NUMELMS(clientDeprecatedList); i++)
clientUpgradeInfoMap[clientDeprecatedList[i].strOldName] = &clientDeprecatedList[i];

for (uint i = 0; i < NUMELMS(serverDeprecatedList); i++)
serverUpgradeInfoMap[serverDeprecatedList[i].strOldName] = &serverDeprecatedList[i];
}

// Query the correct map
SDeprecatedItem* pItem = MapFindRef(bClientScript ? clientUpgradeInfoMap : serverUpgradeInfoMap, strFunctionName);
// Extract just the function name if it's being called
string strCleanFunctionName = strFunctionName;
YelehaUwU marked this conversation as resolved.
Show resolved Hide resolved
size_t parenPos = strCleanFunctionName.find('(');
if (parenPos != string::npos)
YelehaUwU marked this conversation as resolved.
Show resolved Hide resolved
strCleanFunctionName = strCleanFunctionName.substr(0, parenPos);

// Trim any whitespace
strCleanFunctionName.erase(0, strCleanFunctionName.find_first_not_of(" \t\n\r"));
strCleanFunctionName.erase(strCleanFunctionName.find_last_not_of(" \t\n\r") + 1);

// Query the correct map with the cleaned function name
SDeprecatedItem* pItem = MapFindRef(bClientScript ? clientUpgradeInfoMap : serverUpgradeInfoMap, strCleanFunctionName);
if (!pItem)
return ECheckerWhat::NONE; // Nothing found

Expand Down
2 changes: 1 addition & 1 deletion Server/mods/deathmatch/logic/CResourceChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class CResourceChecker
long FindLuaIdentifier(const char* szLuaSource, long* plOutLength, long* plLineNumber);
bool UpgradeLuaFunctionName(const std::string& strFunctionName, bool bClientScript, std::string& strOutUpgraded);
void IssueLuaFunctionNameWarnings(const std::string& strFunctionName, const std::string& strFileName, const std::string& strResourceName,
bool bClientScript, unsigned long ulLineNumber);
bool bClientScript, unsigned long ulLineNumber, const std::string& strContext);
ECheckerWhatType GetLuaFunctionNameUpgradeInfo(const std::string& strFunctionName, bool bClientScript, std::string& strOutHow, CMtaVersion& strOutVersion);
int ReplaceFilesInZIP(const std::string& strOrigZip, const std::string& strTempZip, const std::vector<std::string>& pathInArchiveList,
const std::vector<std::string>& upgradedFullPathList);
Expand Down