-
Notifications
You must be signed in to change notification settings - Fork 387
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
Remove BNPC names from files and use IDs instead #5659
Comments
I can think of at least a few triggers that would break if we did that. I also think it makes triggers/timelines less human readable with only ability IDs to go off of. |
Yeah, those are also my concerns as well re: removing names. I was just enumerating some options that have been brought up in the past for completeness. |
I think that a mapping file that was something like the following would be fine for readability? Could be auto-generated from SaintCoinach exports or via XIVAPI, whichever is easier to wire up? const NpcMapping = {
TiamatsClone: {
en: 'Tiamat\'s Clone',
jp: 'ティアマット・クローン',
// etc
bNpcNameId: 12242,
},
}; Usage: {
id: 'EO 21-30 Tiamat Clone Dark Wyrmwing',
type: 'StartsUsing',
netRegex: { id: '7C65', source: NpcMapping.TiamatsClone.en, capture: false },
response: Responses.goMiddle(),
}, The translation layer in We could probably do something similar for |
That's a nice idea. I do worry some about collisions here, but maybe we could just append the name id in that case, e.g. I also wonder if we could also support The tricky part here is that we'd need to do something for timelines as well, and in some cases you can't auto-generate |
If you're auto-generating these, what's to prevent the same problem of the NpcMapping.[name] changing when there's a rename? What about when the rename is more than just a simple punctuation change (like we saw with Criterion in 6.28)? We can end up in a situation where the triggers refer to an old name that no longer exists, which makes maintenance confusing. |
Yeah, that's awkward. It doesn't happen that often though, and at the very least it will throw TypeScript errors in ~most cases so it should be obvious what needs fixing. That's more than we have now! |
Couldn't we handel bnpc (and maybe actions) like we do for petnames? and if we need to use them just refere to the ids oh the newly generated file? than we wouldn' need to specify enemies in the timeline files at all and just use the generated resources to get the localized name? |
Based on #5645 (comment) (and to have a seperate topic available)
Current issue stated by quis:
Unfortunately there's only bnpcid on the added combatant line but not on other lines.
Suggestions sofar:
quis: Other options here could be that the translation section could have a bnpcid -> name section (like the syncs, but just referenced differently) or alternatively alternatively we could consider dropping names from triggers (and timelines?)
xiashtra suggestion was to ask ravahn to includ bnpcid to all other loglines
I am currently unsure of following things:
The text was updated successfully, but these errors were encountered: