Enum-ify several marked flag/result parameters - #3462
Open
MrKeiKun wants to merge 12 commits into
Open
Conversation
Replaces the raw int flag on chrif_save()/chrif->save() with enum chrif_save_flag (CSAVE_NORMAL, CSAVE_QUITTING, CSAVE_CHANGE_MAPSERV), matching the values already documented in the function's header comment.
Replaces the raw int flag on clif_createchat()/clif->createchat() with enum clif_createchat_flag, matching the values already documented in the function's ZC_ACK_CREATE_CHATROOM packet comment.
Introduces enum notify_ban_errorcode in common/mmo.h with the BAN_* error codes already documented on clif_authfail_fd(), and applies it to both independent implementations: clif->authfail_fd() (map-server) and chr->authfail_fd() (char-server), including their HPM hook plugin defs.
Replaces the raw int flag on unit_walk_toxy()/unit->walk_toxy() with bitmask enum unit_walk_toxy_flag, matching the values already documented in the function's header comment.
Replaces the raw int flag on battle_calc_defense()/battle->calc_defense() with bitmask enum battle_calc_defense_flag (BCD_IGNORE_DEFENSE, BCD_PIERCE_DEFENSE, BCD_TOTAL_DEFENSE_REDUCTION), matching the values already documented in the function's header comment.
Introduces enum login_auth_result in login.h, covering every value documented in login_auth_failed()'s switch statement, and applies it to login_mmo_auth(), login_mmo_auth_new(), check_client_version(), and auth_failed(). Introduces a separate enum loginlog_rcode for the success/ip-banned codes used only by loginlog_log(), which otherwise receives login_auth_result values passed through on failure.
Verified the flag values documented in battle_calc_base_damage()'s header comment against every call site (direct calls and the GET_NORMAL_ATTACK/GET_NORMAL_ATTACK2 macros) and confirmed they match, resolving the TODO's correctness concern. Introduces bitmask enum battle_calc_base_damage_flag for the flag parameter; flag2 (the separate BF_* combat flag already used as plain int across several sibling functions) is left untouched, matching the TODO's original scope.
Retypes Damage.type from plain int to enum battle_dmg_type: every assignment already used the BDT_* named constants, and it flows directly into clif_damage()'s enum battle_dmg_type parameter.
noviewid is a uint32 equip-position bitmask (set via atoi in npc_parse_mapflag), but the live noviewid branch treated it as a plain boolean and cached a valueless revert string, silently discarding the original mask on zone-mapflag revert. A correct implementation already existed further down as dead code, unreachable because the buggy branch matched first; promoted it to the live position instead. Also removes two harmless dead duplicate branches (cvc, src4instance) that were shadowed by earlier identical branches in the same chain.
Resolves the TODO about map_zone_mf_cache() re-comparing the flag string against every known mapflag name on every call: 57 of the 75 mapflags follow an identical boolean on/off revert pattern, so they are now matched via a single name/getter lookup table and a shared helper (map_zone_mf_cache_bool_flag), replacing 57 near-identical strcmpi branches with one table (each getter reads its backing struct map_flag member, preserving the few cases where the mapflag name and its backing field differ, e.g. nopenalty/noexppenalty). The remaining 18 mapflags with parameterized or otherwise non-boolean revert logic (nosave, zone, nocommand, jexp, adjust_skill_damage, the damage-rate flags, etc.) are left as an explicit strcmpi chain, unchanged in behavior. npc_parse_mapflag(), which does the actual mapflag application (as opposed to this function's revert-caching role), is intentionally left untouched -- it mutates live map state and carries additional cross-flag validation, making it a separate, higher-risk refactor.
Introduces enum ItemStackRestrictions in itemdb.h, matching the existing enum ItemNouseRestrictions style, and applies it to both sides of the item stack-limit bitmask: db2sql.c's plugin encoder (resolving its 'FIXME: Use an enum') and itemdb.c's item DB parser, which decoded the same bits from raw literals.
…s.py The mapflag dispatch table refactor (map_zone_mf_cache in map.c) introduced 54 map_zone_mf_get_* getters and map_zone_mf_cache_bool_flag, all private static helpers not meant to be part of the map interface, following the same pattern as the already-whitelisted map_zone_mf_cache_add. Without this, CI's interface validator flags each of them as a method that should be registered on map->.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Prelude
Changes Proposed
chrif_save()/chrif->save()flag →enum chrif_save_flagclif_createchat()/clif->createchat()flag →enum clif_createchat_flagclif_authfail_fd()/chr->authfail_fd()type →enum notify_ban_errorcode(shared across both independent map-server and char-server implementations, including their HPM hook definitions)unit_walk_toxy()/unit->walk_toxy()flag → bitmaskenum unit_walk_toxy_flagbattle_calc_defense()/battle->calc_defense()flag → bitmaskenum battle_calc_defense_flaglogin_mmo_auth()and related helpers (login_mmo_auth_new(),check_client_version(),auth_failed()) →enum login_auth_result, covering every value already documented inlogin_auth_failed()'s switch statementloginlog_log()rcode → separateenum loginlog_rcodefor the success/ip-banned codes not part oflogin_auth_resultbattle_calc_base_damage()flag → bitmaskenum battle_calc_base_damage_flag(the TODO also flagged the values as possibly incorrect; verified against all call sites/macros and confirmed correct)struct Damage.type→enum battle_dmg_type, matching the type already used by every assignment site and byclif_damage()'s parametermap_zone_mf_cache()'s mapflag-name dispatch: replaced 57 near-identicalstrcmpibranches (of 75 total) with a single name/getter lookup table, resolving the TODO about repeatedly re-comparing the flag string on every call. The remaining 18 mapflags with parameterized or non-boolean revert logic are left as an explicit chain;npc_parse_mapflag()(a separate, higher-risk function that mutates live map state) is intentionally untouched.enum ItemStackRestrictions, matching the existingenum ItemNouseRestrictionsstyle, applied to bothdb2sql.c's plugin encoder anditemdb.c's matching decoderflag2onbattle_calc_base_damage()was intentionally left asint, since it's the pre-existingBF_*combat flag shared by several sibling functions that still use plainint— retyping it would be a separate, wider change.Also includes an unrelated but related bug fix found during this work:
map_zone_mf_cache()'snoviewidbranch treated auint32equip-position bitmask as a boolean, silently discarding the mask on zone-mapflag revert; fixed by promoting the correct (but dead/unreachable) implementation that already existed further down in the same function. Two harmless dead duplicate branches (cvc,src4instance) were removed in the same commit.Issues addressed: N/A