Skip to content

further onHear improvements (more in description) #3004

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

further onHear improvements (more in description) #3004

wants to merge 10 commits into from

Conversation

Zbizu
Copy link
Contributor

@Zbizu Zbizu commented May 2, 2020 •
edited
Loading

see #3001 for details

now interacts with

  • received private message
  • received broadcast
  • received yell and whisper

test logs:

(notes: normal message on default)

speaker: Administrator, listener: Administrator, message: hi, type: say
speaker: Administrator, listener: Player, message: hi, type: say

(notes: original message instead of "pspsps" when out of range gets passed as param for the purpose of catching what the player actually said, distance can be checked through lua anyway)

speaker: Administrator, listener: Administrator, message: hi, type: whisper
speaker: Administrator, listener: Player, message: hi, type: whisper

(notes: yell function)

speaker: Player, listener: Administrator, message: I'M YELLING, type: yell
speaker: Player, listener: Player, message: I'M YELLING, type: yell

(notes: can't change the message on sender's end because displaying it is client-sided)

speaker: Player, listener: Administrator, message: hey it's player speaking to administor, type: priv from

(notes: returning false lets the spell cast, the text just doesn't get displayed)

speaker: Administrator, listener: Administrator, message: utevo lux, type: say
speaker: Administrator, listener: Player, message: utevo lux, type: say

(notes: speaker is correct, the message was sent from player character with god account type)

speaker: Player, listener: Administrator, message: red private test ("@username@
message"), type: private red from

(notes: #b message in client)

> Administrator broadcasted: "broadcasting through hasttag b".
speaker: Administrator, listener: Player, message: broadcasting through hasttag
b, type: broadcast
speaker: Administrator, listener: Administrator, message: broadcasting through h
asttag b, type: broadcast

(notes: /b message)

> Administrator broadcasted: "broadcasting through talkaction".
speaker: Administrator, listener: Player, message: broadcasting through talkacti
on, type: broadcast
speaker: Administrator, listener: Administrator, message: broadcasting through t
alkaction, type: broadcast

(notes: npc response to hi)

speaker: The Oracle, listener: Administrator, message: CHILD! COME BACK WHEN YOU
 HAVE GROWN UP!, type: npc from

script used for testing:

local types = {
	[TALKTYPE_SAY] = "say",
	[TALKTYPE_WHISPER] = "whisper",
	[TALKTYPE_YELL] = "yell",
	[TALKTYPE_PRIVATE_FROM] = "priv from",
	[TALKTYPE_PRIVATE_TO] = "priv to",
	[TALKTYPE_CHANNEL_Y] = "channel yellow",
	[TALKTYPE_CHANNEL_O] = "channel orange",
	[TALKTYPE_PRIVATE_NP] = "npc from",
	[TALKTYPE_PRIVATE_PN] = "npc to",
	[TALKTYPE_BROADCAST] = "broadcast",
	[TALKTYPE_CHANNEL_R1] = "channel red",
	[TALKTYPE_PRIVATE_RED_FROM] = "private red from",
	[TALKTYPE_PRIVATE_RED_TO] = "private red to",
	[TALKTYPE_MONSTER_SAY] = "monster say",
	[TALKTYPE_MONSTER_YELL] = "monster yell",
	[TALKTYPE_CHANNEL_R2] = "red 2",

}

function Creature:onHear(speaker, words, type)
	print("speaker: "..speaker:getName()..", listener: "..self:getName()..", message: "..words..", type: "..types[type])
	
	return true
end

@Zbizu Zbizu mentioned this pull request May 2, 2020
Closed
@EvilHero90
Copy link
Contributor

The problem with this entire thing is, I intentionally didn't implement onHear to be like this because this will be a bottleneck for the server, imagine that every message which is generated ingame will be checked now and even multiply times (due to listeners) I've did performence tests years back with similiar codes to this and the performence was horrible.

You could basicly compare this to getSpectators function, this takes a huge toll aswell and so will this but if you provide performence tests (server with atleast 100+ players) and the tests are good then we can talk again about this idea.

@DSpeichert DSpeichert added the feature New feature or functionality label May 3, 2020
@Zbizu
Copy link
Contributor Author

Zbizu commented May 4, 2020 •
edited
Loading

What exactly should I be paying attention to when testing?

Do you think that bool onSay would be better?

@yamaken93
Copy link
Member

Performance wise its bad as @EvilHero90 said. Its way better to just register an event(creaturescripts) on the players/creatures that need the event.

@MillhioreBT
Copy link
Contributor

Performance wise its bad as @EvilHero90 said. Its way better to just register an event(creaturescripts) on the players/creatures that need the event.

It is a good idea

@Zbizu
Copy link
Contributor Author

Zbizu commented May 6, 2020

technically we are calling getSpectators already. Nobody had concerns when onMove or onMoveItem were added? They get executed way more often than onHear

@MillhioreBT MillhioreBT mentioned this pull request May 8, 2020
Merged
@Zbizu
Copy link
Contributor Author

Zbizu commented Nov 9, 2020

Closing it because I won't be working on it anytime soon.

Reasons for closing:

  • the solution I provided is inefficient
  • I don't plan to work on it in near future
  • I'm doing a cleanup in my local fork

@Zbizu Zbizu closed this Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature New feature or functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Zbizu @EvilHero90 @yamaken93 @MillhioreBT @DSpeichert