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

Codex voucher redemptions are not handled #1888

Closed
Tkael opened this issue Jul 9, 2020 · 8 comments
Closed

Codex voucher redemptions are not handled #1888

Tkael opened this issue Jul 9, 2020 · 8 comments
Labels
6. not up to spec The expected behaviour per the spec doesn’t match the observed behaviour. needs thought For when you need to slow down and consider things.

Comments

@Tkael
Copy link
Member

Tkael commented Jul 9, 2020

EDDI version in which issue is found

3.7.0

VoiceAttack version in which issue is found (as applicable)

N/A

Steps to reproduce

  1. Redeem a voucher from a codex discovery

Expected

EDDI to provide a scripted response for the voucher redemption

Observed

The following is recorded in the log:

2020-07-08T21:42:32 [Warning] JournalMonitor:ParseJournalEntry Unhandled voucher type codex { "timestamp":"2020-07-08T21:42:32Z", "event":"RedeemVoucher", "Type":"codex", "Amount":50000, "Faction":"" }

Investigation

The "RedeemVoucher" journal entry includes a "Type" property. EDDI currently creates different "voucher redeemed" style events for each "Type" that it supports. It does not currently support a "codex" type and consequently the "codex" type is unhandled.

if (type == "bounty")
{
events.Add(new BountyRedeemedEvent(timestamp, rewards, amount, brokerpercentage) { raw = line, fromLoad = fromLogLoad });
}
else if (type == "CombatBond")
{
events.Add(new BondRedeemedEvent(timestamp, rewards, amount, brokerpercentage) { raw = line, fromLoad = fromLogLoad });
}
else if (type == "trade")
{
events.Add(new TradeVoucherRedeemedEvent(timestamp, rewards, amount, brokerpercentage) { raw = line, fromLoad = fromLogLoad });
}
else if (type == "settlement" || type == "scannable")
{
events.Add(new DataVoucherRedeemedEvent(timestamp, rewards, amount, brokerpercentage) { raw = line, fromLoad = fromLogLoad });
}
else
{
Logging.Warn("Unhandled voucher type " + type, line);
}

We have several options to resolve this:

  1. Fold the "codex" type into the already existing "Data voucher redeemed" event.
  2. Create a separate and new Codex voucher redeemed event.
  3. Consolidate all of the "voucher redeemed" style events into a single event and single script, controlling the variations on script output via a "type" property which would be included in the consolidated event.
@Tkael Tkael added 6. not up to spec The expected behaviour per the spec doesn’t match the observed behaviour. needs thought For when you need to slow down and consider things. labels Jul 9, 2020
@nepomuk16321
Copy link

Thank you for the hints and the opening of this little problem.
I think your number one is the easiest way to implement the codex voucher.

@Tkael
Copy link
Member Author

Tkael commented Jul 12, 2020

@richardbuckle @Hoodathunk @GioVAX Are we in agreement that option 1 should be our approach?

@GioVAX
Copy link
Contributor

GioVAX commented Jul 12, 2020

#1 is indeed the easiest way to implement this, but it would not be consistent with the existing code structure.

If we have time, I'd go for #3.

@Tkael
Copy link
Member Author

Tkael commented Jul 13, 2020

I think option 3 is my preference too (it'll be the cleanest overall and most of the current voucher redeemed scripts already follow similar patterns, but it's also the most disruptive).

@richardbuckle
Copy link
Member

Let's step back and ask ourselves what is the use case of having different event types here?

I would say implement (1) right away and then maybe create another ticket to migrate to (3) if we decide that is what we want to do.

@Tkael
Copy link
Member Author

Tkael commented Jul 13, 2020

That's just it - I don't really see the use case for having multiple fairly redundant events and swapping to a single event makes it easier to add new voucher redemption options if new voucher types are added in future updates.

@richardbuckle
Copy link
Member

Very well. let's implement (1) and create another ticket to migrate to (3).

@Tkael
Copy link
Member Author

Tkael commented Oct 25, 2020

Ticket for option 3 generated, #1980.

@Tkael Tkael closed this as completed in 3689160 Oct 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6. not up to spec The expected behaviour per the spec doesn’t match the observed behaviour. needs thought For when you need to slow down and consider things.
Projects
None yet
Development

No branches or pull requests

4 participants