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

Effects spawning #299

Merged
merged 42 commits into from
Jul 2, 2022
Merged

Effects spawning #299

merged 42 commits into from
Jul 2, 2022

Conversation

TheDuckCow
Copy link
Member

No description provided.

@TheDuckCow TheDuckCow linked an issue May 15, 2022 that may be closed by this pull request
3 tasks
@TheDuckCow TheDuckCow added this to the v3.4.0 milestone May 15, 2022
@TheDuckCow TheDuckCow changed the base branch from master to dev May 15, 2022 02:30
Also moves to stricter enum matching for references to avoid typos. Placeholders for other effects spawner types.
Want to ensure we don't accidentally change this file unless intentional, tracking using lfs enables this
Allows to optionally yet generally define a way to set inputs for particular node inputs, and have multiple presets to define an effect while reusing the same geometry nodes. Sample file checked in, but will be updated after getting feedback from geonodegroup owner on the ideal settings
Also moves to stricter enum matching for references to avoid typos. Placeholders for other effects spawner types.
Want to ensure we don't accidentally change this file unless intentional, tracking using lfs enables this
Allows to optionally yet generally define a way to set inputs for particular node inputs, and have multiple presets to define an effect while reusing the same geometry nodes. Sample file checked in, but will be updated after getting feedback from geonodegroup owner on the ideal settings
@TheDuckCow TheDuckCow changed the base branch from dev to master July 1, 2022 03:11
@TheDuckCow TheDuckCow changed the base branch from master to dev July 1, 2022 03:11
@TheDuckCow
Copy link
Member Author

Hey @StandingPadAnimations - no obligation, but if you have time and would like to, would love to have you glance through and see if things look good from your standpoint. If you don't have time no worries, I'll just merge.

# -----------------------------------------------------------------------------


def create_auto_footfall_particle_plane():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like this doesn't do much. It may be best to comment this out so Python doesn't have to create a function object

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair call - better to make it an enhancement issue vs deadcode, removed in the meantime.


# Importer helper
filter_glob = bpy.props.StringProperty(
default="*.png;*.jpg;*.jpeg;*.tiff",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a way to take advantage of the EXTENSIONS variable for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I've made the update now.

@@ -1513,6 +1519,161 @@ def model_spawner(self):
# Test collection/group added
# Test loading from file.

def geonode_effect_spawner(self):
"""Test the geo node variant of effect spawning works."""
if bpy.app.version < (3, 0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need a maxBv function similar to the minBv function added alongside the optimizer. It has a benefit of also being cacheable, something that would help a lot

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch to ask about this function. I don't think a maxBv is needed, as we can take any min bv and just inver the logic. You can see the change I made in this commit.

Copy link
Collaborator

@StandingPadAnimations StandingPadAnimations left a comment

Choose a reason for hiding this comment

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

Most of it seemed pretty good, though there are a few things I noticed

@StandingPadAnimations
Copy link
Collaborator

if you don't mind, I'll be pushing a few commits to this branch

@StandingPadAnimations
Copy link
Collaborator

Actually most of the changes should be added once this is merged (mainly within util.py)

@TheDuckCow
Copy link
Member Author

Thanks for the review @StandingPadAnimations, super nice to get another set of eyes and feedback on these things. I'm going to merge in the meantime to dev.

@TheDuckCow TheDuckCow merged commit f1016f0 into dev Jul 2, 2022
@TheDuckCow TheDuckCow deleted the effects-spawning branch July 2, 2022 04:35
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.

Weather and Effects Spawning
2 participants