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

Corpse Finder #960

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Corpse Finder #960

wants to merge 24 commits into from

Conversation

Fluboxer
Copy link
Contributor

basically this thing
изображение
support sharing cords in chat. Support parsing cords from chat (skyhanni format). Shows corpse moment even a pixel of it gets on your screen

todo: make it parse skyblocker format

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Aug 25, 2024
@AzureAaron AzureAaron added the new feature This issue or PR is a new feature label Aug 26, 2024
@AzureAaron AzureAaron added this to the 1.23.0 milestone Aug 28, 2024
@AzureAaron AzureAaron added the merge conflicts This PR has merge conflicts that need solving. label Sep 3, 2024
@LifeIsAParadox LifeIsAParadox removed the merge conflicts This PR has merge conflicts that need solving. label Sep 8, 2024
@Fluboxer
Copy link
Contributor Author

Fluboxer commented Sep 8, 2024

pretty sure I clicked wrong thing but eh ima go see if it compiles and works

@Fluboxer
Copy link
Contributor Author

Fluboxer commented Sep 8, 2024

tested, it works

Copy link
Contributor

@olim88 olim88 left a comment

Choose a reason for hiding this comment

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

feature seems to work well. with a few nitpicks in the code:

int z = Integer.parseInt(matcher.group("z"));
LOGGER.info(PREFIX + "Parsed message! X:{}, Y:{}, Z:{}", x, y, z);
boolean foundCorpse = false;
BlockPos parsedPos = new BlockPos(x-1, y-1, z-1); // skyhanni cords format difference is -1, -1, -1
Copy link
Contributor

Choose a reason for hiding this comment

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

is skyhanni the only other mod to send cwords? surely this would mess up detecting cwords from this mods messages. Would it be better just to see if they are within 2 blocks distance to a location instead of relying on this conversion?

lockPos parsedPos = new BlockPos(x, y, z);
...
...
if(corpse.waypoint.pos.getSquaredDistance(parsedPos) < 4) {

Copy link

Choose a reason for hiding this comment

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

If you're talking in terms of the temporary waypoint that gets added when someone sends a location, the way I wrote it wouldn't cause an issue. it would just add the corpse location to a list so it doesn't get shared since someone already shared it's location, parsing the waypoint was using skyhanni patcher coords waypoint feature

Copy link

Choose a reason for hiding this comment

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

tho reading it again, you'd probably want to use a range of blocks rather than checking corpse location, when I wrote it on SH I didn't see any other mod that had it like this so I just sent the corpse location, but other mods might share the player location.
and also some people just don't realise features like this exist so they just /patcher sendcoords

@nobaboy
Copy link

nobaboy commented Nov 1, 2024

If you'd like help since I originally wrote it I don't mind, tho I'm writing it in kotlin, tho it looks like you're done

@Fluboxer
Copy link
Contributor Author

Fluboxer commented Nov 2, 2024

I'm kinda sick rn so can't do much, brain isn't braining
If anyone want you can "suggest changes" and I can just get them in

@nobaboy
Copy link

nobaboy commented Nov 2, 2024

Pretty much the only thing is, check in a radius around the corpse like olim88 said, it's a small and niche thing but it'd be better that way

@AzureAaron
Copy link
Collaborator

This needs to be rebased as its not going to work on 1.21.2+

@AzureAaron AzureAaron added merge conflicts This PR has merge conflicts that need solving. and removed reviews needed This PR needs reviews labels Nov 22, 2024
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge conflicts This PR has merge conflicts that need solving. labels Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This issue or PR is a new feature reviews needed This PR needs reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants