-
Notifications
You must be signed in to change notification settings - Fork 380
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
eureka: 5.57 notes, array support for notes #3166
base: main
Are you sure you want to change the base?
Conversation
5.57 patch notes included Dyunbu's note (12) to drop in 2 additionals skirmishes, one which already had a drop so I added it as an array and added a typeof check to support both. Hasn't been tested because of maintenance.
@@ -3621,7 +3622,9 @@ class EurekaTracker { | |||
// Adds field note drops, name, id & rarity of those | |||
if (this.zoneInfo.treatNMsAsSkirmishes && this.options.EnrichedSTQ && nm.fieldNotes) { | |||
for (const note of fieldNotesList) { | |||
if (note.id === nm.fieldNotes) | |||
if (typeof nm.fieldNotes === 'string') | |||
nmNote = [note]; |
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.
Where is nmNote
declared?
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.
oh wow
I need to some doing PRs while half-asleep
if (note.id === nm.fieldNotes) | ||
if (typeof nm.fieldNotes === 'string') | ||
nmNote = [note]; | ||
if (note.id in nmNote) | ||
enriched.innerHTML = `#${note.id}: ${note.shortName} ${gRarityIcon.repeat(note.rarity)}`; |
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.
Does this just overwrite the previous text if there are two notes?
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 was pondering whether to add it on the same line or to make it smaller, not sure yet. It can get kinda long but the user can choose to disable enriched content anyway, so maybe we should go with 2 on the same line ? only happens for a single skirmish anyway
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.
Multiple lines is probably ok!
Sorry I haven't gotten back to this, but I was waiting for you to test/respond to comments above. Were you planning on looking at this some more? |
Hello, didn't have time to manage this for now, if you want to take over please do, I don't wanna drag it for too long ! |
5.57 patch notes included Dyunbu's note (12) to drop in 2 additionals skirmishes, one which already had a drop so I added it as an array and added a typeof check to support both.
Hasn't been tested because of maintenance.