search

Justw8 / NameplateSCT

Add Personal SCT to NameplateSCT
MIT License
22 stars 20 forks source link

Building on overkill with configs #13

Closed mike-robertson closed 3 years ago

mike-robertson commented 3 years ago

The goal of this pull request is to build on the overkill code created by @EnDscx by putting it behind a toggle. I may also add additional configurations to allow offset positioning of the overkill text.

mike-robertson commented 3 years ago

The goal of this pull request is to build on the overkill code created by @EnDscx by putting it behind a toggle. I may also add additional configurations to allow offset positioning of the overkill text.

Sweet. I set the default to disabled as well. Since you are receptive to this, I may take some time to try to add in config options for placement as well in a follow up pull request. Just didn't want to waste any time learning that/trying it out if you weren't interested in this functionality. Appreciate the quick responses as well!

mike-robertson commented 3 years ago

@Justw8 just an FYI before merging this, I am pretty sure there is some kind of defect with the base overkill code I built on top of. Got a few lua errors where it was trying to concat a null value in a string - I assume some kind of scenario where the overkill is null? I may have to do some checking on how to guard against that scenario

edit: actually, it may be that the school is undefined. I noticed in other parts of the code it is guarding against that. I will add that in.

mike-robertson commented 3 years ago

Ok, I think I have a fix pushed up for that. I was able to do some testing and killing with an auto attack is what was causing the lua error. I tested after the fix and it seemed to display correctly. I also took the liberty of slightly refactoring the color code you had already so I could just reuse that.

You are going to want to double check my work here since I'm basically googling this + using existing code to understand the syntax & hoping lua linter doesn't let me down.

Justw8 commented 3 years ago

Testing this out on the 9.1 PTR.

Justw8 commented 3 years ago

This is in the alpha now.