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

Add termials #101

Merged
merged 15 commits into from
Mar 17, 2025
Merged

Add termials #101

merged 15 commits into from
Mar 17, 2025

Conversation

Aras14HD
Copy link
Contributor

@Aras14HD Aras14HD commented Mar 14, 2025

I implemented termials including fractional termials and chains.
The results can be exact, approximate, digits and tower.

In the end, I removed both Calculation enums, as it made more sense to keep it as a factorial level 0.
As I ran into a dedup issue with those chains, I reworked that, so that it dedups based on capture.

Further:

  • Improved fallback for termial of too large approximate (digits), currently falsely no-op
  • fix name mistake terminal->termial

Resolves #53

@tolik518
Copy link
Owner

Currently I fear that it would spam the existing subreddits* with replies. Especially the major subreddits like mathmemes and theydidthemath would get spammed by little factorion.
So my suggestion would be to either

  • have a ruleset by subreddit
  • calculate terminals only on summon
  • calculate terminals only on command
  • ...?

I'm not sure what the solution should be and I'd like to hear what you think :)

*mathmemes, mathmemescirclejerk, unexpectedfactorial, factorialchain, doublefactorialchain, theydidthemath, theydidthemonstermath

@Aras14HD
Copy link
Contributor Author

Why not all? Check for terminals if it's a direct mention or in a whitelisted subreddit or explicitly enabled via command.

All those sound pretty easy (might also improve mention parent check behaviour while I'm at it). While command is the least invasive, that would make it go pretty much fully unused, so the whitelist makes sense (especially for unexpectedfactorial) and then I might as well add the mention exception too.

@Aras14HD
Copy link
Contributor Author

Aras14HD commented Mar 14, 2025

Thought: Should this perhaps also apply to subfactorials?

@tolik518
Copy link
Owner

We also could use all of the suggestions, sure. The only argument against is the effort.

We could use use that for subfactorials, you're right, but subfactorials are very rarely used, since it's hard to make an subfactorial on accident (compared to factorials and terminals)

add terminal whitelist
add terminal on mention
add terminal on command
add terminal of approximate math function
refactor CalculationJob::execute to reduce code duplication
fix mixed chain calculation
fix order of tower levels
@Aras14HD
Copy link
Contributor Author

Added those rules, so subreddit whitelist (TERMINAL_SUBREDDITS env var), mentions and command ("terminal" and "triangle").

Also improved the accuracy of tower fallback (for terminals) and improved that code. (Also noticed that I was wrong about including repeated logs in nested towers, so removed that)

@tolik518
Copy link
Owner

Give me a heads up when the PR is ready for review :)

@Aras14HD Aras14HD requested a review from tolik518 March 16, 2025 10:03
@Aras14HD
Copy link
Contributor Author

Should be all good now, can't think of anything I need to do :)

@tolik518
Copy link
Owner

I just figured out we both were using "terminal", instead of "termial", lol.

@Aras14HD
Copy link
Contributor Author

Oops ^_^' gonna fix that later

@Aras14HD Aras14HD changed the title Add terminals Add termials Mar 17, 2025
@Aras14HD Aras14HD requested a review from tolik518 March 17, 2025 13:50
@tolik518
Copy link
Owner

Looks good, i’m excited to see it in the wild.
Also I figured out that there should be a flag or something to disable the mentions - otherwise its a bit annoying to run locally as local factorion then tries to reply to all the mentions :D

@tolik518 tolik518 merged commit e2cf84b into tolik518:master Mar 17, 2025
4 checks passed
@Aras14HD Aras14HD deleted the terminal branch March 18, 2025 06:33
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.

Triangular number/ Termial function support
2 participants