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

Some CrateSpawner related changes #15589

Closed
wants to merge 3 commits into from

Conversation

MustaphaTR
Copy link
Member

@MustaphaTR MustaphaTR commented Sep 5, 2018

First commit fixes the small issue with owners i noted at #15586.

Second commit adds an actor init to pass which CrateSpawner trait spawned the crate, so multipile CrateSpawner traits can have their max-min values separetly calculated. I never noticed if it was ever a problem, nor tested it. But if that was actually the case, this should fix preplaced and Supply Truck (and Scrap Crates for Generals Alpha) crates intervene with CrateSpawner trait.

Third commit makes the trait conditional. While not much useful under World actor. I use CrateSpawner under Player actor and i used conditions so i can add additional crates per player in No Bases mode i added to my mod. GrantConditionOnPrerequisite unfortunately doesn't work on World actor. If it did, we could remove the dedicated lobby option logic under CrateSpawner and use LobbyPrerequisiteCheckbox instead. On my engine i moved the crate lobby option stuff to MapOptions: trait but i still think best way of doing it is LobbyPrerequisiteCheckbox, so haven't touched it here. I can update this PR if someone can help me with that.

@reaperrr
Copy link
Contributor

Needs rebase.

@MustaphaTR
Copy link
Member Author

Rebased.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

From IRC: Using crate types would be better than using an init.

@pchote
Copy link
Member

pchote commented Sep 30, 2018

The reason I suggested crate types is that it can then take into account manually placed crates on the map / from lua / etc.

@MustaphaTR
Copy link
Member Author

As i said on IRC:

pchote, abcdefg30 : about that CrateSpawner PR, while it would be good to somehow allow preplaced crates to count towards the trait. I don't think checking the types is a good idea. If i define CrateSpawner on an normal/Player actor, i would expect it to apply Min/Max per player/actor/trait which wouldn't be the case if it was done with types.

@pchote
Copy link
Member

pchote commented Oct 4, 2018

What if we keep the trait on the player actor, then add a flag that multiplies the defined min/max by the current player count?

@MustaphaTR
Copy link
Member Author

If we are gonna multipile with player count, i don't think we need to move the trait to player actor.

Another idea i have is to have a seperate manager trait to check the amount of crates on the map and CreateSpawner trait spawns them accordingly.

As i said in the main comment, getting GrantConditionOnPrerequisite on world actor could really help here by getting rid of Specific Lobby options and use LobbyPrerequisiteCheckbox instead.

@pchote
Copy link
Member

pchote commented Mar 9, 2019

@MustaphaTR are you planning to continue with this? If not, we should probably close this.

@pchote
Copy link
Member

pchote commented Apr 21, 2019

Closing as stale.

@pchote pchote closed this Apr 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants