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

feat: Add blackfroest scenario #1332

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Bienenstock
Copy link

I hope there are no bugs left, I created this scenario a long time ago.
Have Fun!

Copy link
Collaborator

@grilledham grilledham left a comment

Choose a reason for hiding this comment

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

Cool, thanks for making the PR.

I had a quick look and this mostly seems fine.

One thing I did notice is if I start the map with _DEBUG = true I get this error.
image

Comment on lines 208 to 210
score_cave_collapses=Cave collapses
score_mine_size=Mine size
score_experience_lost=Experience lost
Copy link
Collaborator

Choose a reason for hiding this comment

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

These 3 are missing from the German translation. Any reason for that?

Copy link
Author

Choose a reason for hiding this comment

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

ups, idk, I created it a long time ago. i don't think they are used. I will check tomorrow

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@Bienenstock
Copy link
Author

Cool, thanks for making the PR.

I had a quick look and this mostly seems fine.

One thing I did notice is if I start the map with _DEBUG = true I get this error. image

I have a look tomorrow

@Bienenstock
Copy link
Author

Cool, thanks for making the PR.

I had a quick look and this mostly seems fine.

One thing I did notice is if I start the map with _DEBUG = true I get this error.

fixed

@Bienenstock Bienenstock requested a review from grilledham August 28, 2022 02:15
Copy link
Collaborator

@grilledham grilledham left a comment

Choose a reason for hiding this comment

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

There are some warnings in the luacheck. The "cyclomatic complexity of function on_tick is too high" might be hard to fix but the others are probably worth looking at. I can bypass the checks if we think it's not worth the effort but probably worth a look first.


for rock_index = rock_count, 1, -1 do
local rock = rocks[rock_index]
raise_event(defines.events.on_entity_died, {entity = rock})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I ran into an issue with trying to raise the on_entity_died event. Diggy uses defines.events.script_raised_destroy here instead.

@jacobwatkinsgit
Copy link
Contributor

I have a bunch of patches but I can't commit them because git wants write permissions to someone elses repository.

@Bienenstock Here, before I throw my computer out the window:

Summary:
add destroy_tree to create_particles to make leaf-particles, the way destroy_rock creates stone-particles, fix function description (create_particle, not create_entity)

refactor black_forest_grow: iterates every tree on the map, doesn't error on invalid entities any more, checks every tree and reduces redundant tree checks

general code quality cleaning, like using entity.destroy{raise_destroy = true}, instead of manual script raised destroy + an empty destroy().

changed_files.zip

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.

3 participants