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

Added stats generation #1089

Merged
merged 2 commits into from
Nov 16, 2024
Merged

Added stats generation #1089

merged 2 commits into from
Nov 16, 2024

Conversation

brwarner
Copy link
Contributor

@brwarner brwarner commented Nov 16, 2024

Checklist

  • The new code additions passed the tests (npm test).
  • The linter ran and found no issues (npm run-script lint).
  • Check the ink repository for any tests to migrate surrounding stats

Description

Adding stats generation from the ink compiler to inkjs. The ink compiler only exposes this as a command line option, but for inkjs I've exposed it as a new function in the Compiler that can be run after compilation.

For source from the ink repository, see https://github.com/inkle/ink/blob/master/compiler/Stats.cs

@brwarner brwarner marked this pull request as ready for review November 16, 2024 04:27
@brwarner
Copy link
Contributor Author

There are no tests in the ink repository around story stats

@smwhr
Copy link
Collaborator

smwhr commented Nov 16, 2024

Thanks for porting this stat counter !

Although adding one simple test case for the function to insure no one breaks it unintentionally in the future would be a nice addition, don't you think ?

@brwarner
Copy link
Contributor Author

For sure, I'll add one in!

@brwarner
Copy link
Contributor Author

Tests are in src/tests/specs/inkjs/compiler/Stats.spec.ts

@smwhr smwhr merged commit c050af1 into y-lohse:master Nov 16, 2024
1 check passed
@smwhr
Copy link
Collaborator

smwhr commented Nov 16, 2024

Merged ! 🚀

@smwhr
Copy link
Collaborator

smwhr commented Nov 16, 2024

The number of diverts is not counted in the same manner as when called with the original inklecate.

Ex, on the file src/tests/inkfiles/original/inkjs/tests.ink

  • this implementations reports 101 diverts
  • the inklecate implementation reports 131 diverts

This has nothing to do with your code, so I will just open a new issue for later resolution.
Also, who is really interested in the number of diverts ?

@brwarner brwarner deleted the stats branch November 17, 2024 20:34
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.

2 participants