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

[ fields.lua ] Update 0xC8. #2338

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

[ fields.lua ] Update 0xC8. #2338

wants to merge 2 commits into from

Conversation

eLiidyr
Copy link
Contributor

@eLiidyr eLiidyr commented Mar 1, 2024

Update flags for types.alliance_member in 0x0C8.

06:01 - 2-bit value is the Index of the party the player is in.
06:03 - Bit is set if the player is a party leader.
06:04 - Bit is set if the player is the alliance leader.
06:05 - 2-bit value indicating quartermaster. First bit is 1 if you are in an alliance with quartermaster set.
06:07 - Bit is set if the player is a linkshell owner in linkshell slot 1.
06:08 - Bit is set if the player is a pearlsack owner in linkshell slot 1.

Thanks to @KenshiDRK for some additional info about the flags.

Update flags for `types.alliance_member` in `0x0C8`.
@z16
Copy link
Member

z16 commented Mar 1, 2024

While the PR is good and the data is useful, this will break the code for anyone using the current Flags field. I don't know how much that is currently used, if at all, but that's something I'll need to think about for a bit. I will probably still accept it, because the changes are good by themselves, and it should not be difficult switching over for anyone who is using it. Just something I'll sleep on first.

@RubenatorX
Copy link
Collaborator

RubenatorX commented Mar 1, 2024

I have a couple of questions, and some proposed changes. Mostly dealing with clarity of value meaning.

Party Index --
a) Would like to add a comment that the value is either 0, 1, or 2
b) Thoughts on changing the label to something mildly more specific? Just to drill home that it is the alliance party index, and not, the index of their slot in the party? (noting that, simply doing "a" will help with this as well so, possibly not necessary)

Quartermaster --
petition to split this into two fields.
one indicating quartermaster is on.6
and another indicating this player is the quartermaster.
because grouping them into one field seems incorrect otherwise if this is indeed true First bit is 1 if you are in an alliance with quartermaster set. -- because that indicates 2 flags, not one two bit number.

Linkshell #? ??? --
First, it is a tad unclear even with these comments what this means. Could you clarify @eLiidyr?
Bit is set if the player is a ? owner in linkshell slot 1.
I'm assuming that this is intended to mean that the player has any shell in their slot #1.
But, it is also open to interpretation/assumptions, such as, they are a ?holder in __my__ linkshell #1
So, I think that this would benefit from
a) A further clarified comment in the file
And possibly also a
b) A label change.
Both/either possibly using the verbage shellholder/shell owner/linkshell owner and/or sackholder/sack holder. e.g.s LS#1 Shellholder/Linkshell #1 Shellholder or Player LS#1 Is Shell/Player LS\#1 Shellholder or Slot 1 Shellholder/Slot 1 Is Shell

Also, my 2 cents is go ahead and break usage of .Flags -- this is better

@z16
Copy link
Member

z16 commented Mar 1, 2024

Party Index --
a) Would like to add a comment that the value is either 0, 1, or 2
b) Thoughts on changing the label to something mildly more specific? Just to drill home that it is the alliance party index, and not, the index of their slot in the party? (noting that, simply doing "a" will help with this as well so, possibly not necessary)

Not opposed to a), but it's kinda obvious if you look at the type, since it can only hold values 0, 1, 2 and 3, so it cannot be the index of the player within the party. Not necessarily opposed to b), but do you have a suggestion? I can't really think of anything better. We have several fields with the name structure X Index and it almost always refers to the index of X inside the outer container. The only exception to that is Inventory Index, and we're not really consistent with that either, we do still call it Item Index, Crystal Index, etc. in various situations. (And personally I'd like to change the remaining examples of Inventory Index as well, but these are too commonly used to justify that, and honestly, in this case it's kinda unambiguous what it means.)

@eLiidyr
Copy link
Contributor Author

eLiidyr commented Mar 1, 2024

@RubenatorX

Would like to add a comment that the value is either 0, 1, or 2.

None: 00, Party 1: 01, Party 2: 10, Party 3: 11
I can add these to the comments.

Thoughts on changing the label to something mildly more specific...

As for the labels, it is a party identifier, not which member slot in the party you are. Personal preference to all of them really, as I find your wording confusing as well. Party just seems to refer to Grouping to me, and Party Member would refer to a singular player.

Petition to split this into two fields.

This personally leads back to your last complaint, as then you have a flag that is now only available when quartermaster is set, and when in an alliance, and to me personally, there is no label to easily identify that; would you not agree?
I mean I guess you can have two flags, Alliance Quartermaster & Party Quartermaster

One indicating quartermaster is on.

It doesn't indicate this, which is actually wrong. If you set Quartermaster this would not flag anything.

Edit: To further clarify, one is "Only while in alliance", and the other is "the player" who is quartermaster. The alliance flag is also only set on the player who has quartermaster while in the alliance, and there for seems more logical to use it as a value as more of a quartermaster mode, than a flag.

First, it is a tad unclear even with these comments what this means.

In-game, you have a Linkshell 1 & Linkshell 2. these flags have no bearing on secondary linkshell, and thus Linkshell 1, refers to only the linkshell in the first slot; the only one which creates an indicator in the nameplate.
As for the label, there was no indication that a linkpearl effects this, it was only sack/shell.

I'm assuming that this is intended to mean that the player has any shell in their slot #1.

I chose the name scheme to follow the items names in game. You can't just have a "any shell". You have a Linkpearl, Pearlsack, or a Linkshell. Linkpearl has no bearing as it changes no data, thus I used Pearlsack, and Linkshell indicating which item effects the flag.

@RubenatorX
Copy link
Collaborator

RubenatorX commented Mar 1, 2024

None: 00, Party 1: 01, Party 2: 10, Party 3: 11 I can add these to the comments.

Uh, is a None Party used (only?) for when they leave? 🤔
If so, that would be a good thing to note

Party just seems to refer to Grouping to me

I'm not questioning that. What is potentially unclear is whether it is an index of the party (A party slot within an alliance) or an index in the party (a player slot within a party). (note that, obviously it's the former because it's only 2 bits, but I'm trying to think like a Dev for which that wouldn't have occurred to them)

then you have a flag that is now only available when quartermaster is set, and when in an alliance, and to me personally, there is no label to easily identify that; would you not agree? I mean I guess you can have two flags, Alliance Quartermaster & Party Quartermaster

It doesn't indicate this, which is actually wrong. If you set Quartermaster this would not flag anything.

I may need a better explanation of what these values mean then.
I have a few points of confusion now.

  1. Correct me if I am wrong but, is this packet not only used when you're in an alliance?
    I was under the impression that the party version of these packets was 0x0DD, but am I mistaken and does 0x0C8 get used as well for non-alliance party updates as well?

  2. I mean I guess you can have two flags, Alliance Quartermaster & Party Quartermaster
    regardless of the answer to #1, I interpreted this:

2-bit value indicating quartermaster. First bit is 1 if you are in an alliance with quartermaster set.

to mean that the first bit meant that quartermaster was on someone in the alliance at all, and that the second bit meant that this indicated player is the one that is the alliance quartermaster.

just saw this ⬇️ as well so editing my response (before I post it xD)

Edit: To further clarify, one is "Only while in alliance", and the other is "the player" who is quartermaster. The alliance flag is also only set on the player who has quartermaster while in the alliance, and there for seems more logical to use it as a value as more of a quartermaster mode, than a flag.

So you're saying that 0 = not quartermaster, 1 = party quartermaster, 2 = N/A(can't happen), and 3 = alliance quartermaster?

In-game, you have a Linkshell 1 & Linkshell 2. these flags have no bearing on secondary linkshell, and thus Linkshell 1, refers to only the linkshell in the first slot; the only one which creates an indicator in the nameplate. As for the label, there was no indication that a linkpearl effects this, it was only sack/shell.

I was not stating that slot1/slot2 were likely to be confused, I was stating that it wasn't as clear as it could be that the data had only to do with their linkshell slot 1 and not my linkshell slot 1 -- meaning, a comment could help clarify that it's just whether their slot1 link* is a shell or a sack, and has nothing to do with them being in my slot 1 linkshell. Hence why I am suggesting possibly using verbage that indicates that they are a "sackholder" -- which is a "group authority" term rather than a "item name" term. Anyway, I don't have strong feelings about it, just a thought.

As for the label, there was no indication that a linkpearl effects this, it was only sack/shell.

I'm assuming that "they have a linkpearl in slot 1", has the same value as "they do not have any link* equipped at all in slot 1"

And as a side comment... I'm trying to figure out why this data is even in here at all XDD (although it would be far from the first time that there was pointless extra info included in a packet)

@KenshiDRK
Copy link
Member

And as a side comment... I'm trying to figure out why this data is even in here at all XDD (although it would be far from the first time that there was pointless extra info included in a packet)

Thats exactly what I thought when I found about it, and still have no idea why its there.

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.

None yet

4 participants