search

Wynntils / Wynntils-Legacy

Wynntils is a Wynncraft Mod that seeks to enhance the user's gameplay with a variety of customizable options and additions.
https://wynntils.com
GNU Affero General Public License v3.0
157 stars 90 forks source link

refactor: applying minor caching of variables in a few files. #599

Closed TheAvonian closed 2 years ago

TheAvonian commented 2 years ago

Very minor changes to assure no null accesses (even though it shouldn't be possible in these places) and caching SocialData in several methods where there is heavy usage of PlayerInfo.get(SocialData.class).

TheAvonian commented 2 years ago

I fixed the spacing, I forgot to reformat before I committed at the time. Also changed out the generic data names into the more specific ones suggested.

kristofbolyai commented 2 years ago

I am wondering though.. Was there a crash related to these being null? Where did the idea of making these specific methods null-safe come from?

TheAvonian commented 2 years ago

The idea more so came from me working on another feature and realizing in a lot of files there is heavy use of PlayerStats.get(T) within the same method bodies so I went through those ones I was using and just made them try to call it once/twice at most in the body as every call to it is a foreach loop that is unneeded. Honestly the null checks are unneeded additions based on how everything is setup as it seems like they could never be null at those points but I added it for the sake of assurance.

Ideally I would do this to every file but I only made these changes on my open files at the time.

kristofbolyai commented 2 years ago

Yeah, this feels a bit paranoid. But then, the design of those "getters" are kinda bad imo.. So it should be fine to keep it.