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

Use freeze time to calculate value of dynamic challenge #2322

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Typhlos
Copy link

@Typhlos Typhlos commented Jun 9, 2023

Solves #2320

I used the code used in the scoreboard to edit the function calculate_value and filter the solves using freeze time and only keep the solves before freeze time.

Let me know if there is anything missing.

@ColdHeat
Copy link
Member

ColdHeat commented Jun 9, 2023

I think this leads to the inverse of the original problem. For example if we freeze time and then the event continues to run, if we then unfreeze the challenge values will be the frozen values until their next recalculation.

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #2322 (b815af1) into master (23cdf85) will decrease coverage by 0.08%.
The diff coverage is 57.14%.

❗ Current head b815af1 differs from pull request most recent head d2635dc. Consider uploading reports for the commit d2635dc to get more accurate results

@@            Coverage Diff             @@
##           master    #2322      +/-   ##
==========================================
- Coverage   87.56%   87.48%   -0.08%     
==========================================
  Files         146      146              
  Lines        8899     8919      +20     
==========================================
+ Hits         7792     7803      +11     
- Misses       1107     1116       +9     
Impacted Files Coverage Δ
CTFd/utils/challenges/__init__.py 91.42% <33.33%> (-8.58%) ⬇️
CTFd/api/v1/config.py 95.83% <60.00%> (-1.10%) ⬇️
CTFd/plugins/dynamic_challenges/__init__.py 97.01% <85.71%> (-1.35%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Typhlos
Copy link
Author

Typhlos commented Jun 9, 2023

I'll try to see if it's not possible to recalculate all values on unfreeze.

And I'll fix the linting

@Typhlos
Copy link
Author

Typhlos commented Jun 13, 2023

@ColdHeat I modified the code so that on any modification of freeze time, all values are recalculated

@Typhlos
Copy link
Author

Typhlos commented Jun 23, 2023

@ColdHeat please tell me if there's anything missing in the PR

@ColdHeat
Copy link
Member

Instead of having a special case on freeze, I think a better strategy would be to have a list of plugin functions that will be called when config is updated. This way the dynamic value plugin (or other plugins) can register a function to be called after config is updated.

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.

None yet

2 participants