Default all localized behavior to European logic #22
No reviewers
Labels
No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: metin2/server#22
Loading…
Reference in New Issue
No description provided.
Delete Branch "Tr0n/server:localization/default-gf-eu"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Removes all localized behavior in favor of what was enabled for EU.
Only exception to this is the 6th and 7th bonus, which was disabled for EU.
Includes removal of Chinese 3 and 5 hour limits.
g_iUseLocale
andLC_Is*
calls were removed entirely.Thanks, I'll check it out when I have time!
@ -1099,36 +1099,6 @@ bool CArena::RegisterObserverPtr(LPCHARACTER pChar)
bool CArenaManager::IsLimitedItem( int lMapIndex, DWORD dwVnum )
If this function always returns false, shouldn't we better remove it altogether?
@ -870,3 +864,2 @@
addPacket.dwLevel = 0;
}
//TODO: what is this doing here?
How weird! I suppose it can be removed, including that goto.
@ -124,4 +115,0 @@
int iVal;
if (!g_iUseLocale)
iVal = std::min(GetPoint(POINT_SP_RECOVERY), GetMaxSP() * 7 / 100);
How weird that these two branches of the "if" were exactly the same!
@ -2102,3 +2051,1 @@
percent = 8;
else
percent = 9;
percent = 9;
Isn't percent always set to 9 here? I suppose we could just remove the if and leave
percent = 9;
@ -2182,4 +2127,0 @@
// -----------------------
if (LC_IsGermany() && pAttacker && pAttacker->IsPC())
{
int iDmgPct = CHARACTER_MANAGER::instance().GetUserDamageRate(pAttacker);
So I suppose some players in Germany could've had extra damage as a premium feature? How broken and P2W!
@ -146,24 +146,6 @@ bool IS_SUMMONABLE_ZONE(int map_index)
return true;
}
bool IS_BOTARYABLE_ZONE(int nMapIndex)
I wonder what this was used for...
I see, in the YMIR locale you could only open private shops in map1/2. This reminds me of the current behaviour on present day Gameforge servers, where you can only open shops in the Bazaar.
do you want me to revert this?
Honestly, it's fine this way. If somebody ever needs this feature,
CHARACTER::__OpenPrivateShop()
would be a much better to implement it anyway.@ -2611,3 +2488,1 @@
bi = b2;
else
bi = b1;
LuckyBagInfo * bi = b1;
b1
could be renamed tobi
, I suppose, therefore removing this pointer?sure. will do.
@ -3803,10 +3664,6 @@ bool CHARACTER::UseItemEx(LPITEM item, TItemPos DestCell)
case 71051 : // 진재가
{
// 유럽, 싱가폴, 베트남 진재가 사용금지
I see, these were the 6/7 bonuses being banned here.
@ -402,13 +380,7 @@ bool CHARACTER::LearnGrandMasterSkill(DWORD dwSkillVnum)
static bool FN_should_check_exp(LPCHARACTER ch)
Function always returns true, therefore it can be removed.
I will remove it entirely. Thought it would be good to still have the provision to implement own logic.
But you're right coding for what you "might do" in the future is not good.
@ -2269,3 +2234,1 @@
ch->ChatPacket(CHAT_TYPE_COMMAND, "mall http://metin2.co.kr/04_mall/mall/login.htm");
return;
}
char country_code[3];
I'll refactor this sometime in the future, in order to provide a secure authentication mechanism to the CMS I'm working on.
@ -394,4 +390,0 @@
return 0;
}
if (g_iUseLocale) // 중국에서는 금덩어리, 금열쇠, 은열쇠 나오지 않게 함
Shouldn't this have been kept??
g_iUseLocale
is true for Europe.absolutely, my bad.
@ -538,12 +538,9 @@ void CGuildManager::DeclareWar(DWORD guild_id1, DWORD guild_id2, BYTE bType)
if (g1->DeclareWar(guild_id2, bType, GUILD_WAR_SEND_DECLARE) &&
g2->DeclareWar(guild_id1, bType, GUILD_WAR_RECV_DECLARE))
{
if (false == LC_IsGermany())
Why did they disable this in Germany, lol!?
@ -442,3 +434,2 @@
}
const TAccountTable & c_rAccountTable = d->GetAccountTable();
d->Packet(&packFailure, sizeof(packFailure));
I think there's an issue here, it looks like
d->Packet(&packFailure, sizeof(packFailure));
should have remained, including the return statement and everything else. Only after the closing bracket wouldconst TAccountTable & c_rAccountTable = d->GetAccountTable();
follow.good catch, thanks. will revert.
@ -653,4 +617,3 @@
}
else
{
SPDLOG_WARN("VERSION : NO CHECK");
I see you've removed these warnings as well. I guess they aren't of much use after all.
that was by accident. will restore.
@ -723,3 +712,3 @@
if (pinfo->type == CHAT_TYPE_SHOUT)
{
const int SHOUT_LIMIT_LEVEL = g_iUseLocale ? 15 : 3;
const int SHOUT_LIMIT_LEVEL = 15;
It would also be nice to have this configurable
@ -2444,3 +2420,1 @@
{
GuildCreateFee = 200000;
}
GuildCreateFee = 200000;
This should be configurable as well!
@ -1073,4 +1035,0 @@
}
//
// 승룡곡 1층, 2층에서만 7,8 스킬입문서 드롭
Interesting, I wonder if these "7/8" skills were these beta ones: https://metin2.dev/topic/20822-ymir-beta-skills/
@ -6,8 +6,6 @@ typedef std::map< std::string, std::string > LocaleStringMapType;
LocaleStringMapType localeString;
int g_iUseLocale = 0;
I would have kept this variable and used it for English, so as in locale_find() to skip checking the locale string map.
While I think the point for having a locale file for English to English being pointless is fair, I also believe it to be valuable to have all locales being treated the same.
That way you would be "forced" (not really if you don't care about errors in the logs) to keep your locale_string up to date with all messages you used in the code.
Therefore you have a much easier time down the line, when you do decide to localize all messages, as you don't have to look through the code for all your custom messages: you should have added them to the locale_string from the beginning.
Your argument is fair and I've thought of that as well. The thing is that once the multilanguage system will be implemented, this behaviour will have to be reverted, as the English strings will be converted to
[LS;id]
strings as Mali showcases in his topic on metin2.dev: https://metin2.dev/topic/29790-official-client-locale-stringreversed/Therefore, the errors will show up in the log if a translation string ID is not configured in the server.
Not to mention that the 2024 agile software development good practices™ (patent pending) thing to do would be to have a CI job to check for this kind of issues. But as the saying goes here, we'll eat a bread or two by the time that happens.
All good, since you're way more involved in the m2 dev scene I will definitely follow your judgement here 👍
@ -34,3 +27,3 @@
SPDLOG_ERROR("LOCALE_ERROR: \"{}\";", string);
return s_line;
return string;
I don't get what happened here and what warrants the change from s_line to string?
my bad. Was testing something locally and forgot to revert. Will restore original functionality.
@ -479,4 +419,0 @@
g_setQuestObjectDir.clear();
g_setQuestObjectDir.insert("locale/germany/quest/object");
g_stLocaleFilename = "locale/germany/locale_string.txt";
I think we should better keep these init functions, so as for example locale_string_de.txt is loaded when German is specified, locale_string_fr.txt for French, and so on and so forth...
i restored them and made the English locale the default. Let me know if that implementation fits what you had in mind.
Yeah, this works for now as a stop-gap solution until the multilanguage system is implemented in the client side.
@ -683,4 +680,0 @@
if (teen_addr[0] && teen_port)
g_TeenDesc = DESC_MANAGER::instance().CreateConnectionDesc(ev_base, dns_base, teen_addr, teen_port, PHASE_TEEN, true);
extern unsigned int g_uiSpamBlockDuration;
I think the spam-related stuff should have remained?
Probably. My mind just went to "if we're not an auth server, we must be a teen server" and yeeted the branch. will revert.
@ -1357,3 +1357,3 @@
// 파티 결성 후 충분한 시간이 지나면 경험치 보너스를 받는다.
if (!m_iLongTimeExpBonus && (get_dword_time() - m_dwPartyStartTime > PARTY_ENOUGH_MINUTE_FOR_EXP_BONUS * 60 * 1000 / (g_iUseLocale?1:2)))
if (!m_iLongTimeExpBonus && (get_dword_time() - m_dwPartyStartTime > PARTY_ENOUGH_MINUTE_FOR_EXP_BONUS * 60 * 1000 / 1))
I suppose we can get rid of the division by 1.
was unsure if that maybe is used to convert float to int (and if that matters here).
will remove the
/1
@ -2063,69 +2063,7 @@ teleport_area:
// 3: 이미 같은 이름이 사용중
// 4: 성공
// 5: 해당 기능 지원하지 않음
if ( LC_IsEurope() )
I don't know how to feel about this. In recent updates, you can now change your name on European servers as well. Should we really delete this feature?
I will revert the removal. See I haven't played official in so long that I didn't know you can change your name at all.
@ -5442,4 +5264,0 @@
LogManager::instance().CharLog(this, gold, "DROP_GOLD", "");
}
if (false == LC_IsBrazil())
I just noticed this, the right branch that should've been kept is the one with 150 seconds.
You're right. Gotta love when people mix
true == IsSomething
,false == IsSomething
and all the negated variations 😅I know that feel, I just hope I didn't miss any either, my head hurts after reading so much source code xD
I suppose the
item->StartDestroyEvent(150);
thing in char_item.cpp is the only thing left before I can merge thisCool, thanks for the thorough review! If you have anything in particular you would like help with, just let me know.
No worries, I also made a discord server, even though there isn't much discussed in there: https://discord.gg/UwR7fXzE