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

Add pet_service library #460

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

Conversation

sigilbaram
Copy link
Contributor

Adds a service that tracks pet information.

@sigilbaram sigilbaram changed the title Pet service Add pet_service library Jul 10, 2021
@sigilbaram sigilbaram mentioned this pull request Jul 10, 2021
@sigilbaram
Copy link
Contributor Author

See #461 for related library.

Copy link
Member

@z16 z16 left a comment

Choose a reason for hiding this comment

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

I often tell people to split their PRs up into different PRs for each logical unit, I believe this is the first time I think two PRs should have been one :D The two are logically the same unit, one is just the interface, the other is the implementation. Oh well, a bunch of changes you need to do, for other things feel free to contact me in the Discord server.

libraries/pet_service/pet_service.lua Outdated Show resolved Hide resolved
libraries/pet_service/pet_service.lua Outdated Show resolved Hide resolved
head = {item_type},
frame = {item_type},
attachments = {item_type[12]},
available_heads = {struct.bitfield(4)},
Copy link
Member

Choose a reason for hiding this comment

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

I'll have to think about this some more, but I think it would make most sense to make this an array of booleans, then overwrite it in the library to return a set of item IDs. Feel free to discuss this on Discord.

mp_percent = {struct.uint8},
tp = {struct.uint32},
active = {struct.bool},
automaton = {struct.struct({
Copy link
Member

Choose a reason for hiding this comment

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

I dislike this structure being here when no automaton is present. This should be made a pointer and set to a different automaton struct when an automaton is summoned, and set to nil when it is dismissed. This way players could check if an automaton is present with if pet.automaton then.

Unfortunately, I don't think we have a code example for this yet, let me know if you need help implementing it.

magic = {struct.uint16},
magic_max = {struct.uint16},
str = {struct.uint16},
str_modifier = {struct.uint16},
Copy link
Member

Choose a reason for hiding this comment

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

We should probably discuss the naming here, we don't have this elsewhere yet (though we should add this to the player library as well). In the packet definitions we use stats_base and stats_bonus, but I'm open to suggestions. It should just be the same everywhere.

libraries/pet_service/pet_service.lua Show resolved Hide resolved
libraries/pet_service/pet_service.lua Show resolved Hide resolved
libraries/pet_service/pet_service.lua Outdated Show resolved Hide resolved
data.automaton.chr = p.chr
data.automaton.chr_modifier = p.chr_modifier

local active = data.active and (data.name == data.automaton.name)
Copy link
Member

Choose a reason for hiding this comment

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

I haven't examined this myself, but it feels a bit brittle. Is that the best way to check for an active pet? Can data.automaton.name ever be something other than data.name or empty? If we switch to a pointer based approach this should be moved to the top and the function should just return if it's false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This compares the automaton's name (pet.automaton.name) to the active pet's name (pet.name) to tell if the automaton is the active pet or not.

This is the only way I managed to think of to check this given the known packet fields, but there might be a better way to tell that we just haven't exposed yet. The client somehow knows the automaton is active and will say "You cannot customize an active automaton." if you try to change attachments. I'm skeptical SE actually left this up to something like comparing the name, but I've looked at the known pet relate packets and I don't see anything else in them that would tell the client the active pet is the PUP automaton...


data.automaton.active = active

if p.hp_max ~= 0 and active then
Copy link
Member

Choose a reason for hiding this comment

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

And if we do the pointer based approach, this (and the following) check can be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up just removing these in b0a3112. I originally did this because there is a slight minimum delay between each 0x044 (about 1sec?). Thinking on it more these detailed HP/MP auto stats seem better left as accurate, if slightly stale, rather than being estimated from low precision percents. This way addons can pick if speed or precision are more important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants