-
Couldn't load subscription status.
- Fork 64
util: add basic support for CEs to encounter_tools #729
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
base: main
Are you sure you want to change the base?
util: add basic support for CEs to encounter_tools #729
Conversation
|
@jacob-keller Thanks for your contribution! 🌵🚀 |
|
@valarnin do you think something like this could be merged? I tried to figure out a way to make the encounter listing work without breaking existing support. |
|
I guess my question is whether this approach is good, or if we need to do some other refactoring on the encounter logic as a whole to properly support CEs. |
|
I have some more thoughts on this, and I will try to clean it up before making it a non-draft |
|
I'll hold off on reviewing actual code changes in detail until you switch to non-draft. As far as I've been able to determine, the mapping of
Unfortunately, XIVAPI has a bug with this data sheet where the // normal typedef for XIVAPI exported data
const data: DynamicEventSetType = {
'3.5': {
// ... other fields
'Name': {
'en': 'Calamity Bound',
// other langs
},
},
} as const;
export default data;
export type CEMap<CEList> = {
[directorCEID: string]: keyof CEList;
};
// `<foo>CEs` auto-generated based on XIVAPI sheet
// `<foo>CEs` keys auto mapped by stripping non-alphanumeric characters
// and lowercasing first character maybe?
// `<foo>CEMap` is a manual mapping in the util script, like in `zone_overrides.ts`
export const bozjaCEs = {
// entries from `data` starting with `1.`
} as const;
export const bozjaCEMap: CEMap<typeof bozjaCEs> = {};
export const zadnorCEs = {
// entries from `data` starting with `2.`
} as const;
export const zadnorCEMap: CEMap<typeof zadnorCEs> = {};
export const southHornCEs = {
// entries from `data` starting with `3.`
'calamityBound': data['3.5'],
} as const;
export const southHornCEMap: CEMap<typeof southHornCEs> = {
'32F': 'calamityBound',
};Other solutions are fine too, the ultimate goal is to source the name of the CE and its base "zone" from XIVAPI somehow. |
Any idea if XIVAPI stuff is open source and I could look at what it takes to fix this? |
It's fixed as of a minute ago or so. https://v2.xivapi.com/api/sheet/DynamicEventSet/3:1 For future reference, the XIVAPI discord is at https://discord.gg/MFFVHWC |
|
I'll take a stab at implementing something hopefully in the next couple of days |
6d9744d to
e38d07b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do a more proper review later or tomorrow, just wanted to address this first.
util/xivapi.ts
Outdated
| }`; | ||
| if (maxRow > 0) | ||
| url += `&after=${maxRow}`; | ||
| url += `&after=${maxRow}:${maxSubRow}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be passing a subrow ID if the sheet doesn't have subrow IDs. This works for now but it's unintended behavior.
Probably maxSubRow should be number | undefined, and this should instead be:
if (maxRow > 0) {
url += `&after=${maxRow}`;
if (maxSubRow !== undefined)
url += `:${maxSubRow}`;
}or something along those lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix.
The DynamicEventSet sheet contains both a row_id and a subrow_id. If we query the sheet with "after=3" it returns all events which have a row_id of 3 or greater *and* which have a subrow_id of greater than 0. To properly limit the returned rows, we need to use "row:subrow" syntax with the after field. Extend the logic to obtain the subrow_id and add it to query, ensuring that we can safely iterate over the DynamicEventSet sheet in a following change. Currently appending ":0" to regular after=N "works", this is likely not intentional and should not be relied upon. Instead, only append the subrow indicator if the subrow_id exists. Signed-off-by: Jacob Keller <[email protected]>
Signed-off-by: Jacob Keller <[email protected]>
In preparation for adding proper CE support to the encounter tools, first start generating the encounter data we need. Unfortunately it is currently unknown how the game generates the DirectorUpdate IDs used in the network log messages, but we can at least pull the translated names of the encounters from xivapi. Add a new gen_ce_info_and_maps.ts generator script which will generate several files including: * resources/bozja_encounters.ts * resources/bozja_ce_map.ts * resources/zadnor_encounters.ts * resources/zadnor_ce_map.ts * resources/south_horn_encounters.ts * resources/south_horn_ce_map.ts I opted to split by zone since that will be useful both to limit including only the relevent CEs in a given trigger file, and verify that the zone is correct when we see a relevant network log message in the encounter tools. This does lead to a lot of extra files. I wasn't sure how to combine the file results given the way xivapi.writeFile() works. The list of DirectorUpdate IDs is maintained as part of ce_overrides.ts, which I tried to structure in a similar way as the zone_overrides.ts. Hopefully we can re-use that style if we need to override any data from the API in the future. I have not made any attempt to support pulling the other translations for the Chinese and Korean languages yet. I also only pulled the names, since none of the other data was very relevant for the encounter tools. Suggested-by: Valarnin <[email protected]> Signed-off-by: Jacob Keller <[email protected]>
This is a basic initial attempt at supporting critical encounters and critical engagements from the encounter tools. It relies on the CE information we recently started generating from the xivapi, as well as the DirectorUpdate IDs pulled by hand. It does appear to work properly at least as far as finding encounters go. It does at least log the encounter name based off its English translation. There is likely more work required to verify that no other encounter logic breaks. Additionally, we might want to improve the make_timeline support to use the 0x21 log line as an entry check, since this is what we do in the timeline files that support critical encounters. This code was originally based on work from Valarnin, but its been heavily modified to now pull the CE data from the xivapi. This version only accepts CEs that match the zone we're in so it should have minimal chance of breaking other flows. I did end up hard coding the association of zone to encounter list, which could possibly be improved somehow. Signed-off-by: Jacob Keller <[email protected]>
e38d07b to
e573741
Compare
|
I have some additional followup work to extend the make_timeline.sh with support for inserting appropriate headers for the CE start, as well as a new --timeline_offset commandline argument to allow adjusting the initial timelinePosition value to make it easier to create CE timelines in the way that we have them laid out in the exploration zone timeline files. I'm planning to submit those after this merges. |
|
I think it worked for what I provided in #736 ? |
Legends0 reports that the forked tower critical encounter ID is 33B. Add this to the overrides value so that the generated files get updated. Also add it to the zone file, since we don't yet use the resources files from there yet. Signed-off-by: Jacob Keller <[email protected]>
|
Updated to include the director ID for forked tower reported by Legends0. |
|
Does this need any changes to be acceptable? |
|
Some additional notes... there are more action control lines within forked tower: blood:
|
Add some basic support for detecting and splitting encounters by CE, based on the 0x21 network log line. The IDs for this are manually determined, but we now pull the encounter names from xivapi, and generate appropriate mappings.
This works ok in my tests, and so far doesn't appear to break any other encounter logic. However, it likely needs more testing and fine tuning.
I also now added logic to pull the CE info from xivapi, with a new generator script. I did pull a bit more data than we currently use (since the current code doesn't use anything but the english names). I don't yet pull the Chinese or Korean data like how zone_ids.ts does yet.
There's probably some further cleanup we can do around encounter support, but I wanted to try and get the very basic splitting functionality done.
Signed-off-by: Jacob Keller [email protected]