Skip to content
This repository has been archived by the owner on Aug 8, 2024. It is now read-only.

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

Merged
merged 4 commits into from
Oct 10, 2022
Merged

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

merged 4 commits into from
Oct 10, 2022

Conversation

TheAvonian
Copy link
Contributor

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).

ryanzhoudev
ryanzhoudev reviewed Sep 30, 2022
View reviewed changes
Copy link
Member

@ryanzhoudev ryanzhoudev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things I noticed while skimming through this quickly. I will take another look in the morning or some other time

src/main/java/com/wynntils/modules/core/events/ServerEvents.java Outdated Show resolved Hide resolved
src/main/java/com/wynntils/modules/core/events/ServerEvents.java Outdated Show resolved Hide resolved
src/main/java/com/wynntils/modules/core/events/ServerEvents.java Outdated Show resolved Hide resolved
src/main/java/com/wynntils/modules/core/events/ServerEvents.java Outdated Show resolved Hide resolved
src/main/java/com/wynntils/modules/core/managers/PartyManager.java Outdated Show resolved Hide resolved
src/main/java/com/wynntils/modules/core/managers/PartyManager.java Outdated Show resolved Hide resolved
@TheAvonian
Copy link
Contributor Author

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
kristofbolyai approved these changes Sep 30, 2022
View reviewed changes
Copy link
Member

@kristofbolyai kristofbolyai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems okay. Thanks.

@kristofbolyai
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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.

@ryanzhoudev ryanzhoudev merged commit 7af5f79 into Wynntils:development Oct 10, 2022
DevScyu pushed a commit that referenced this pull request Oct 11, 2022
@TheAvonian @DevScyu
refactor: applying minor caching of variables in a few files. (#599)
e1154a0 
* refactor: applying minor caching of variables in a few files.

* refactor: fixing spacing issues and changing generic data names
DevScyu added a commit that referenced this pull request Oct 11, 2022
@DevScyu @ryanzhoudev
@magicus @byBackfish @kristofbolyai @HighCrit @TheAvonian @Incompleteusern
update branch (#608)
59ea392 
* chore: upgrade gradle to 7.5.1 (#591)

* fix: rework party management gui (#590)

Co-authored-by: Magnus Ihse Bursie <mag@icus.se>

* feat: add focus bar (#586)

Co-authored-by: byBackfish <maik@bybackifsh.de>
Co-authored-by: Magnus Ihse Bursie <mag@icus.se>
Co-authored-by: Kristof Kovacs <49001742+kristofbolyai@users.noreply.github.com>

* feat: custom trade market sell amount buttons (#584)

* feat: Adds 3 custom sell amount buttons to the trade market sell gui, pressing them will put the amount set in the config instantly in chat. Also remade most of the `TradeMarketOverlay`. Fixes various bugs

* fix: add empty line before the custom sell amount info

* fix: remove debug & set the item count to the correct amount (visually)

* fix: save the parsed int instead of parsing it twice

* fix: change the lore to fit the custom sell buttons better

* make the custom buttons appear even if only 1 of the item in your inventory

Co-authored-by: byBackfish <maik@bybackifsh.de>
Co-authored-by: Magnus Ihse Bursie <mag@icus.se>
Co-authored-by: Kristof Kovacs <49001742+kristofbolyai@users.noreply.github.com>

* feat: ability tree navigation using scroll wheel (#582)

* feat: add ability to scroll to navigate through ability tree

* fix: add ability to invert controls

* Fix formatting

* Fix formatting again

* fix: add `ABILITY_TREE_PATTERN` to properly check if the gui is the ability tree, fix order of the settings

* fix: extract slot numbers, change config order to not have duplicates

* fix: remove `/ 120`

* fix: change to non-capturing regex group

* fix: use `ScrollDirection` as type for the `abilityScrollDirection` config option

* make the `shouldAbitlityScroll` config option default to true

Co-authored-by: byBackfish <maik@bybackifsh.de>
Co-authored-by: Magnus Ihse Bursie <mag@icus.se>
Co-authored-by: Ryan <57310593+DonkeyBlaster@users.noreply.github.com>

* feat: add currently active shaman mask display (#577)

* feat: add current mask overlay

* fix: refactor, fix location

* fix: mask not resetting correctly if only one mask ability is equipped

* fix: add option to change display text for each mask individually

* fix: implement reviews

* fix: change the tabulation

* fix: simplify mask detection

* fix: add spaces after `ìf`

* fix: save the current shaman mask in `CharacterData.java` instead of in `CurrentMaskOverlay.java`

* fix: extract mask parsing from CurrentMaskOverlay to OverlayEvents

* add empty line at the end of the file

Co-authored-by: byBackfish <maik@bybackifsh.de>
Co-authored-by: Magnus Ihse Bursie <mag@icus.se>
Co-authored-by: Kristof Kovacs <49001742+kristofbolyai@users.noreply.github.com>
Co-authored-by: Ryan <57310593+DonkeyBlaster@users.noreply.github.com>

* ci: update releases (#592)

* ci: update releases

* ci: allow version update in stable to automatically be pushed to development (untested)

* chore(release): v1.12.1-beta.0 [skip ci]

* ci: fix release names [skip ci]

* fix: mythic detection & /guild list spam (#594)

* fix: make the mythic detection work again, filter out the results of `/gu list` when wynntils runs it automatically

* fix: fix imports

* fix: simplify `AIR` check

* fix: add comment describing the mythic found code

Co-authored-by: byBackfish <maik@bybackifsh.de>

* chore(release): v1.12.1-beta.1 [skip ci]

* feat: add corrupted bar (#595)

* feat: add corrupted bar

* fix: change symbol, add hide default bar

Co-authored-by: byBackfish <maik@bybackifsh.de>
Co-authored-by: Magnus Ihse Bursie <mag@icus.se>

* chore(release): v1.12.1-beta.2 [skip ci]

* fix: ObjectivesOverlay: Fix patterns (#601)

* chore(release): v1.12.1-beta.3 [skip ci]

* fix: hide token during registration process (#603)

* chore(release): v1.12.1-beta.4 [skip ci]

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

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

* refactor: fixing spacing issues and changing generic data names

* chore(release): v1.12.1-beta.5 [skip ci]

* fix: Fix log spam (#600)

* Fix log spam

* Better fix?

* Update src/main/java/com/wynntils/webapi/WebManager.java

Co-authored-by: Scyu_ <scyu@scyu.dev>

* chore(release): v1.12.1-beta.6 [skip ci]

Co-authored-by: Ryan <57310593+DonkeyBlaster@users.noreply.github.com>
Co-authored-by: Magnus Ihse Bursie <mag@icus.se>
Co-authored-by: byBackfish <38255215+byBackfish@users.noreply.github.com>
Co-authored-by: byBackfish <maik@bybackifsh.de>
Co-authored-by: Kristof Kovacs <49001742+kristofbolyai@users.noreply.github.com>
Co-authored-by: Conventional Changelog Action <conventional.changelog.action@github.com>
Co-authored-by: HighCrit <35771251+HighCrit@users.noreply.github.com>
Co-authored-by: Aven <29378966+TheAvonian@users.noreply.github.com>
Co-authored-by: Incompleteusern <58920010+Incompleteusern@users.noreply.github.com>
DevScyu pushed a commit that referenced this pull request Oct 13, 2022
@TheAvonian @DevScyu
refactor: applying minor caching of variables in a few files. (#599)
f918bf8 
* refactor: applying minor caching of variables in a few files.

* refactor: fixing spacing issues and changing generic data names
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ryanzhoudev ryanzhoudev ryanzhoudev approved these changes

@kristofbolyai kristofbolyai kristofbolyai approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TheAvonian @kristofbolyai @ryanzhoudev