Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix and optimizations: kill tracking #3861

Merged
merged 1 commit into from
Dec 18, 2021

Conversation

Znote
Copy link
Member

@Znote Znote commented Dec 18, 2021

Followup on #3860

Bugfix:

  • returns false if it fails to set monsterType (instead of returning nil when it finds it)
  • Player.updateKillTracker now returns status of operation as a bool, instead of void
  • monster:getType() instead of monster.getType()

Optimizations:

  • 1 function instead of 2
  • Invokes getType() 1 time instead of 2 times.
  • Invokes getOutfit() 1 time instead of 6 times.
  • instantiate monsterType as a local
  • no need to confirm that networkMessage is set in conditional statements, since it is created locally
  • no need for elseif, or a conditional statement to determine need to send message to self, since it has already returned something otherwise.

Pull Request Prelude

Changes Proposed

Issues addressed:

Bugfix:
* returns `false` if it fails to set `monsterType` (instead of returning nil when it finds it)
* `monster:getType()` instead of `monster.getType()`

Optimizations:
* 1 function instead of 2
* Invokes `getType()` 1 time instead of 2 times.
* Invokes `getOutfit()` 1 time instead of 6 times.
* instantiate monsterType as a local
* no need to confirm that networkMessage is set in conditional statements, since it is created locally
* no need for `elseif`, or a conditional statement to determine need to send message to self, since it has already returned something otherwise.

local monsterOutfit = monsterType:getOutfit()

local networkMessage = NetworkMessage()
Copy link
Contributor

Choose a reason for hiding this comment

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

we could just use msg, as is done in sources

Copy link
Member Author

@Znote Znote Dec 18, 2021

Choose a reason for hiding this comment

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

Yeah, but msg is ambiguous and could just mean a regular string/text.
Since this is a NetworkMessage with its metamethods, why not call it so? This makes it easier to understand what is going on for players who use this code as reference material to learn how to use NetworkMessage.

@EPuncker EPuncker merged commit b811fe6 into otland:master Dec 18, 2021
@Znote Znote deleted the znote-killtracking branch December 19, 2021 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants