search

multitheftauto / mtasa-resources

This project maintains a list of up-to-date resources that come with Multi Theft Auto.
https://multitheftauto.com
MIT License
151 stars 151 forks source link

killmessages: refactor #332

Closed stoneage-mta closed 3 years ago

stoneage-mta commented 3 years ago

As requested in #331, I was trying to fix the CPU leak problem, but in fact, after some minutes I decided to just rewrite it from scratch, in a much simpler way.

I tested it with this and this was the biggest CPU consumption for me with 15 messages: image (I didn't test with other players, if you wanna help to test contact me in discord)

All parameters of exported functions were left in the same order, to do not break anything, except the function setKillMessageStyle that was removed because I also added the MTA settings system (get) to set up the following settings: displayLines, drawMarginRight, drawMarginBottom, fadeTime, duration, and outputConsole. So now it can be changed using other resources, like admin.

Anyways, please review carefully before merging, because im a little afraid of breaking some existing scripts (or forgetting some other functionality), let me know if you want me to change anything.

Dutchman101 commented 3 years ago

The killfeed isn't centered on right side of screen like before, maybe you should make it match?

It's quite a bit lower.. at least on my resolution 1920 x 1080

Dutchman101 commented 3 years ago

I think this resource deserves a rewrite, mainly because of this CPU usage and also because its code looks a little spaghetti. I can take a look in some hours...

we ought to implement vehicle kill detection for killers in hydra/hunter/rustler/rhino etc.

Originally posted by @ricksterhd123 in https://github.com/multitheftauto/mtasa-resources/issues/331#issuecomment-868689923

^ I agree with that.. for reference, MTA doesn't let you detect the killer/victim inside a vehicle when primary rockets of a vehicle like hydra/hunter are used.

But barely anything is impossible, and this also wasn't. You can work around it by 'guessing' the killer with a smart script, and in practise i've observed an accuracy rate of 99.5%.. it records all vehicle-on-vehicle kills accurately and rarely (if ever) misidentifies a killer/victim.

See example implementation of what im describing in this customized "support hydra / hunter" version of killmessages resource: https://community.mtasa.com/index.php?p=resources&s=details&id=14801

I suggest you use a similar technique to add support for vehicle kills to your refactored killmessages @iDannz1

stoneage-mta commented 3 years ago

See example implementation of what im describing in this customized "support hydra / hunter" version of killmessages resource: https://community.mtasa.com/index.php?p=resources&s=details&id=14801

I added the changes suggested above using your version as inspiration, thank you.

Also, thank you @androksi for helping with testing. (We just forgot to test with Rhino, so please make sure it's working before merging)

ricksterhd123 commented 3 years ago

I will test this later today, thank you.

ricksterhd123 commented 3 years ago

Rhino works nicely

jlillis commented 3 years ago

Has anyone been able to test this in an environment with many players? It all looks good but I can't confirm any performance improvements since it's just me testing locally.

ricksterhd123 commented 3 years ago

I think dutchman101 is running this version on his server. It's been working fine so far as far as I know.

Cheers

Ricky

From: John Lillis @.> Sent: Wednesday, August 4, 2021 8:21:46 PM To: multitheftauto/mtasa-resources @.> Cc: Rick @.>; Mention @.> Subject: Re: [multitheftauto/mtasa-resources] killmessages: refactor (#332)

Has anyone been able to test this in an environment with many players? It all looks good but I can't confirm any performance improvements since it's just me testing locally.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/multitheftauto/mtasa-resources/pull/332#issuecomment-892911551, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AATDAYN7ECK7P4Y6ASAYY53T3GHMVANCNFSM47JWYUBQ.

Dutchman101 commented 3 years ago

Yeah, it's been tested on a large scale without any bugs found - just has a quirk that im not sure of whether it's bad or not.

To best put it into words..

The old killmessages draws new kills in at the top, letting new kills build below the 'first' one - then fading out the oldest ones at the top.

The new killmessages (this PR) draws new kills in at the bottom, making new kills appear above it (towards the top) and additionally also times out ones that are listed for too long, in the absence of new kills to fade them out forcibly.

So, the way killfeed works is sort of reversed.. it's an entirely new flow. It looks good, but im worried about what people got used to.. it will cause confusion. People are used to look at the bottom if they want to see the latest kills. What do we do? Keep feature parity or say it's fine?

stoneage-mta commented 3 years ago

Yeah, it's been tested on a large scale without any bugs found - just has a quirk that im not sure of whether it's bad or not.

To best put it into words..

The old killmessages draws new kills in at the top, letting new kills build below the 'first' one - then fading out the oldest ones at the top.

The new killmessages (this PR) draws new kills in at the bottom, making new kills appear above it (towards the top) and additionally also times out ones that are listed for too long, in the absence of new kills to fade them out forcibly.

So, the way killfeed works is sort of reversed.. it's an entirely new flow. It looks good, but im worried about what people got used to.. it will cause confusion. People are used to look at the bottom if they want to see the latest kills. What do we do? Keep feature parity or say it's fine?

It looks pretty easy to fix, tomorrow morning I can add a new commit making it look like you described, I haven't really realized that this is the order in which the old killmessages work 😅.