Cleaned up the mapflag system #2943
Merged
Conversation
This file contains 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
* Created setter and getter functions. * Adjusted all calls to use these functions. * Cleaned up several functions to be more dynamic to reduce redundancy that was all over the place. * Renamed nosumstarmiracle to nosunmoonstarmiracle. * Adjusted skill_damage mapflag to use proper defined constants. Thanks to @Lemongrass3110 for a lot of help!
aleos89
added
the
status:code-review
|
really like this change, ideally adding a new mapflag is as painless as possible and all these cases are handled dynamically though. |
secretdataz
requested changes
Mar 1, 2018
There was a problem hiding this comment.
Looks better overall.
The struct map_flag inside struct map_data could be converted to a key-value map for a more simpler map-flag adding procedure when we need to add one.
This way we could get rid of all cases in the switch in the getter and setter function on map-flags and only care for ones that need special handling.
src/map/script.cpp
Outdated
| sd->pvp_lost = 0; | ||
| mf = script_getnum(st, 3); | ||
|
|
||
| if( mf <= MF_MIN || mf > MF_MAX ){ |
There was a problem hiding this comment.
Should be mf < MF_MIN or adjust MF_MIN's value since it shares the same value with MF_NOMEMO.
There was a problem hiding this comment.
There are more occurrences of this but I forgot to comment ;w;
src/map/map.cpp
Outdated
| @@ -4319,37 +4319,493 @@ void map_skill_damage_free(struct map_data *m) { | |||
| * @param other Rate to Other target | |||
| * @param caster Caster type | |||
| **/ | |||
| void map_skill_damage_add(struct map_data *m, uint16 skill_id, int pc, int mob, int boss, int other, uint8 caster) { | |||
| void map_skill_damage_add(struct map_data *m, uint16 skill_id, int rate[SKILLDMG_MAX], uint16 caster) { | |||
There was a problem hiding this comment.
Array size has no effect in this context. It will be converted into a pointer anyways.
src/map/script.cpp
Outdated
| sd->pvp_lost = 0; | ||
| mf = script_getnum(st, 3); | ||
|
|
||
| if( mf <= MF_MIN || mf > MF_MAX ){ |
There was a problem hiding this comment.
Should be mf < MF_MIN or adjust MF_MIN's value since it currently shares the same value with MF_NOMEMO, making the flag unavailable.
Thanks to @secretdataz
aleos89
added
the
status:inprogress
* Converted mapflags to C++ map container. * This allows for much more simple and less code than previously required. * Converted drop_list to vector. * Converted skill_damage ERS into vector and increased limit from UINT8 to UINT16. Thanks to @Lemongrass3110!
* Refactored global maps into a vector. * Resolves sketchy memmoves when deleting maps. Thanks to @Lemongrass3110!
aleos89
removed
the
status:inprogress
* Enumerated nightmare mapflag drop types. * Refactored some final direct mapflag adjustments. * Adjusted map ID check to also check for the cap.
* Converted map_setmapflag_sub to return bool. * Cleaned up initialization of mapflag union arguments to C++ standard. * Corrected MF_SKILL_DAMAGE error message.
* Adjusted a few final things. Thanks to @Lemongrass3110!
Lemongrass3110
approved these changes
Jul 18, 2018
aleos89
removed
the
status:code-review
ghost
mentioned this pull request
SeravySensei
pushed a commit
to SeravySensei/rathena
that referenced
this pull request
Jan 26, 2019
* Created setter and getter functions. * Adjusted all calls to use these functions. * Converted mapflags to C++ map container. * Converted drop_list to vector. * Converted skill_damage ERS into vector and increased limit from UINT8 to UINT16. * Cleaned up several functions to be more dynamic to reduce redundancy that was all over the place. * Renamed nosumstarmiracle to nosunmoonstarmiracle. * Adjusted skill_damage mapflag to use proper defined constants. * Refactored map index into a vector. Thanks to @Lemongrass3110 for a lot of help and @secretdataz!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addressed Issue(s): N/A
Server Mode: Pre-renewal and Renewal
Description of Pull Request:
Thanks to @Lemongrass3110 for a lot of help!