Skytils / SkytilsMod

Skytils is a Hypixel Skyblock mod! Be careful, malicious copies are distributed across GitHub. Confirm on discord.gg/skytils (807302538558308352)
https://hypixel.net/threads/3856202/
GNU Affero General Public License v3.0
1.05k stars 424 forks source link

Blaze slayer fixes and features #367

Closed Desco1 closed 1 year ago

Desco1 commented 1 year ago

Fixes blaze slayer features being broken and adds 3 new features to blaze slayer. These are not meant to change how slayer detection or any of the other features work internally to not conflict with the feature/slayer branch. Instead, these are meant to be a temporary solution until SlayerFeatures gets rewritten.

Actual changes:

Note about the Slayer RNGesus Meter: still broken after the chat message changed. While fixable by keeping track of the open gui, checking if it's a slayer RNG Meter, checking which drop is selected and saving how much is the target XP of the drop, I figured that it'd be easier to save and download the XP required for each drop and use the chat message after changing RNG Meter drop to get it instead.

pull-request-quantifier-deprecated[bot] commented 1 year ago

This PR has 169 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!


Quantification details

``` Label : Medium Size : +152 -17 Percentile : 53.8% Total files changed: 3 Change summary by file extension: .kt : +152 -17 ``` > Change counts above are quantified counts, based on the [PullRequestQuantifier customizations](https://github.com/microsoft/PullRequestQuantifier/blob/main/docs/prquantifier-yaml.md).

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a balance between between PR complexity and PR review overhead. PRs within the optimal size (typical small, or medium sized PRs) mean: - Fast and predictable releases to production: - Optimal size changes are more likely to be reviewed faster with fewer iterations. - Similarity in low PR complexity drives similar review times. - Review quality is likely higher as complexity is lower: - Bugs are more likely to be detected. - Code inconsistencies are more likely to be detected. - Knowledge sharing is improved within the participants: - Small portions can be assimilated better. - Better engineering practices are exercised: - Solving big problems by dividing them in well contained, smaller problems. - Exercising separation of concerns within the code changes. #### What can I do to optimize my changes - Use the PullRequestQuantifier to quantify your PR accurately - Create a context profile for your repo using the [context generator](https://github.com/microsoft/PullRequestQuantifier/releases) - Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the `Excluded` section from your `prquantifier.yaml` context profile. - Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your `prquantifier.yaml` context profile. - Only use the labels that matter to you, [see context specification](./docs/prquantifier-yaml.md) to customize your `prquantifier.yaml` context profile. - Change your engineering behaviors - For PRs that fall outside of the desired spectrum, review the details and check if: - Your PR could be split in smaller, self-contained PRs instead - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR). #### How to interpret the change counts in git diff output - One line was added: `+1 -0` - One line was deleted: `+0 -1` - One line was modified: `+1 -1` (git diff doesn't know about modified, it will interpret that line like one addition plus one deletion) - Change percentiles: Change characteristics (addition, deletion, modification) of this PR in relation to all other PRs within the repository.


Was this comment helpful? :thumbsup:  :ok_hand:  :thumbsdown: (Email) Customize PullRequestQuantifier for this repository.

Desco1 commented 1 year ago

Before merging there are a few things that could be considered:

Sychic commented 1 year ago
* RNG Meter display still broken after chat, which could be fixed easiest in my opinion by downloading the required xp for each drop, a few more details in my first message

How did the message change? I haven't done slayer in forever so I have no clue what it is now or when it changed. Did it change from percentage with a bar?

* Feature: open RNG Meter menu after a boss if the selected boss has dropped the selected item. Can easily be done by comparing stored xp and sending the /cb command of the message, but not sure if it could somehow be considered a cheat which is why I haven't added it yet.

We could do something similar to the "open maddox batphone" feature where you just click anywhere instead of clicking on a specific bit.

* Feature: a visual display for every blaze that is affected by the 2x damage from the Moody Grappleshot. Not too useful unless abusing tactical insertion, but might be neat to have? From a bit of testing though, it doesn't seem that there is any easy way to detect when a blaze is being pulled

We could do something where the blaze itself checks if there's the anvil + the lead nearby (there's got to be some invisible entity that the lead is tied to, no clue what it could be tho).

  I don't think either of those are critical enough so merging and then following this pr with another in some future is also fine

Also works for me.

Desco1 commented 1 year ago

RNG meter chat message

Reference. Yes, it now only displays the stored XP, not which drop was selected nor % to it or the next. When the RNG Meter is complete, the drop is shown in chat without magic find, but if it drops by itself, even when selected, the magic find will be shown. More reference. There is also this chat message shown when a drop is selected from the meter gui, which also shows the slayer it is from. All drops have the same XP for every single player, which is why I think it could be downloaded from server at startup. Another alternative could be permanently looking for the RNG Meter gui and grabbing the required XP from there, but it gets truncated when actually selected, which is pretty annoying

RNG meter menu like maddox phone

Thought of that, but I didn't want to deal with the consequences of having both messages at once. The Stored XP message could not show when autoslayer is disabled, although I may have just missed it. Could also make it so a different mouse button is a different menu?

Hooks

When a player uses the hook an S1BPacketEntityAttach gets sent, leash is not 0, vehicleEntityId is the player who used it and entityID is a bat, which most of the times is not in the world yet and its spawn has to be handled in a different event. I've tried recreating vanilla collissions from that bat to blazes but I didn't get any. I have tried looking for the "snap" effect where some blazes teleport slightly to the hook and checking if blazes have the same velocity as the bat (they do in X and Z, but it is always 0); but I didn't get consistent results for that detection. Also, the hook 2x damage doesn't last forever, but that can be easily tested compared to the detection

Sychic commented 1 year ago

RNG meter chat message

We should be able to get all the data when the player is actually clicking (though that would take a bit of trust in hypixel to actually receive the click). It's probably just more reliable to download from the data repo and look for when the item is updated to tell if hypixel actually received the click to update the RNG item. Though this should probably be separate from all these changes.

RNG meter menu like maddox phone

Wdym both messages at once? I don't think there's a specific scenario when you would have both the maddox menu and the rng menu (since the maddox one should be entirely manual from using the batphone). We would just keep the most recent /cb usage and execute that.

Hooks

We could use the bat itself to look for blazes near itself (using Entity#getEntitiesWithinAABBExcludingEntity) and set a flag in them to tell somewhere else to render them differently. Not entirely version agnostic but not really any better way to do it (though adapting mixins should be pretty simple)

Desco1 commented 1 year ago

RNG menu like maddox phone

There isn't a situation where both messages would be active at the same time but I figured it's better to save players from themselves.

In any case, I agree that this PR should not be expanded with more features and instead I'll follow up with the rest of them in some future. Should be ready to merge