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

Not really rewrite mod system #1072

Open
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

SweetSea-ButImNotSweet
Copy link
Contributor

@SweetSea-ButImNotSweet SweetSea-ButImNotSweet commented Jan 2, 2024

Resolve #1064

Screenshot_20240107-192638.jpg

TODO:

  • Rewrite mod system
  • Add configurations
  • Add the introduction of mod patch
  • Test with replay
  • Add a short delay before rendering username

P/S: Playing the replay which applied the mods after the initialization or always during game in older version may lead to desynchronization.

@SweetSea-ButImNotSweet SweetSea-ButImNotSweet changed the title Add mod patch - Fix a long-lasting issue related to mods Fix a long-lasting issue related to mods Jan 6, 2024
@SweetSea-ButImNotSweet SweetSea-ButImNotSweet marked this pull request as ready for review January 7, 2024 11:45
@SweetSea-ButImNotSweet SweetSea-ButImNotSweet changed the title Fix a long-lasting issue related to mods Not really rewrite mod system and log viewer Jan 7, 2024
parts/gameTables.lua Outdated Show resolved Hide resolved
parts/language/lang_en.lua Outdated Show resolved Hide resolved
parts/language/lang_en.lua Outdated Show resolved Hide resolved
parts/player/draw.lua Outdated Show resolved Hide resolved
parts/player/init.lua Show resolved Hide resolved
parts/scenes/app_console.lua Outdated Show resolved Hide resolved
parts/gameTables.lua Outdated Show resolved Hide resolved
Copy link
Member

@Not-A-Normal-Robot Not-A-Normal-Robot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just these two things and I think it should be good

parts/language/lang_en.lua Outdated Show resolved Hide resolved
parts/scenes/app_console.lua Outdated Show resolved Hide resolved
parts/scenes/app_console.lua Outdated Show resolved Hide resolved
parts/gameFuncs.lua Outdated Show resolved Hide resolved
@SweetSea-ButImNotSweet SweetSea-ButImNotSweet marked this pull request as draft January 9, 2024 08:28
@ImpleLee
Copy link
Contributor

I would suggest splitting such a large pull request into multiple pull requests, if possible, so that each pull request can focus on a single functionality and be reviewed separately.

@ImpleLee
Copy link
Contributor

To be honest, I want to just close this pull request, but after all, I am not the one responsible for maintaining this repository, and it is kind of disrespectful to do so, so I would just wait.

Copy link
Member

@Not-A-Normal-Robot Not-A-Normal-Robot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm, but yeah next time I suggest you to split different changes to different PRs

@SweetSea-ButImNotSweet
Copy link
Contributor Author

code lgtm, but yeah next time I suggest you to split different changes to different PRs

I am doing it right now T_T

@SweetSea-ButImNotSweet SweetSea-ButImNotSweet changed the title Not really rewrite mod system and log viewer Not really rewrite mod system Jan 11, 2024
Copy link
Member

@Not-A-Normal-Robot Not-A-Normal-Robot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I skimmed through the code, LGTM

Comment on lines 1012 to 1014
for _=1,20 do
coroutine.yield()
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a TEST.yieldN() thing you could use instead. @MrZ626 would you recommend using that, or is that just a temporary thing?

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.

Classic and GrandMaster EXTRA don't respect the randomizer mod
3 participants