Buff meteor impact event by adding new rare impact size#6684
Buff meteor impact event by adding new rare impact size#6684hhyyrylainen merged 18 commits intoRevolutionary-Games:masterfrom
Conversation
|
Thank you for contributing to Thrive. Before your contribution can be accepted, you need to sign our CLA. Once your contribution has been accepted you can ask to be included in the credits (https://wiki.revolutionarygamesstudio.com/wiki/Team_Members) and you can apply to join the team and use this work as a sample: https://revolutionarygamesstudio.com/application/ |
|
I could see an immediate benefit to increasing the magnitude of the compound addition. But is there really a problem of them happening to infrequently right now? The objective is to have a dynamic variation in circumstances. Events happening too often homogenizes things just as badly as them happening not often enough. Also, what does the impact size actually do again? |
I think it counts how many adjacent patches are affected... @Patryk26g should know more accurately what it does. |
I was already kind of questioning me increasing the chance, but I definitely agree that it's a bad idea now that you brought this reasoning. One sec, I'll revert that. |
This reverts commit f50beb6.
|
Alright, I've signed the CLA! |
As requested by hhyyrylainen
|
Code change looks fine to me now. Needs balance testing next. |
|
I'll try to review and test it later or tomorrow |
|
@Patryk26g So this isn't new to this PR (but I saw it when wondering why the radioactive meteor was only changed in its CO2 addition), but why do the chunks added not have any numerical amount attached to them? Is it just a fixed number? If so, it might make sense to bump that number up in this PR as well. |
What do you mean by this, exactly? |
The only thing that seems to be changed about the radioactive meteor is that it adds more CO2, so it doesn't do anything to add more radioactive chunks than before? |
Ah, I just noticed the chunk keys in |
No, I think you're touching the wrong things now. You're increasing the amount of compound in each chunk. You should revert that last change. I think "density" in that same file is what you're looking for, but you would have to check. |
|
I'm really not on board with meteors creating "super" chunks that are way more powerful than normal ones. Rather I think if anything the density of the chunks should be upped (so that they spawn more frequently). Edit: looks like I was too slow by 2 minutes, so yeah I guess I'm the technical person to confirm that "density" is the value to change rather than the amount in the biome config. |
|
I just played through 15 generations and I saw precisely one meteor (a radioactive one). So I suspect something is not quite working correctly. |
I don't think this is necessary? It would be preferable for the chunks to match the compound buff, but that does not mean the compounds should not be buffed. |
|
|
Maybe I should have asked this at the start, but do the events even need buffing? Patryk just buffed them recently. So this PR is not really even addressing any open issue in the first place... |
There's been sentiments about the events not being very impactful, but I don't remember if @Patryk26g also already buffed the meteors recently.
Yeah, I was aware of that, but not being able to buff chunk compounds does not automatically mean that the other compounds should not be increased. |
I have it linked in the starting PR message, but it comes from a thread made by Deus. |
|
A couple of weeks ago, buffs to all events including meteors: #6603 |
|
This doesn't include the new impact size that my PR now solely consists of, so I just see this as another reason (besides the game breaking with it) to not include compound increases in this. |
|
Tbf my event buffs weren't enough so it's a good idea to buff it even further, especially with the extended reach of meteors. Needs testing tho if they don't got too far with carbon tho |
We can figure that out later with another PR. For now, I just want this to get merged after it's confirmed working. |
IdfbAn
changed the title
|
You can revert the commit with the removal of chunks buff. just remembered that if you buff the compounds outsidenof chubks their density has to remain at most 0.001. Btw, after the event is finished, the level of compounds is decreassed a bit so you can check it out as well. I think that besides buffing density which has a limit you can also buff the amount |
|
Okie dokie, as you wish, it's your PR after all! |
|
I wouldn't want to merge this if the CO2 add amounts are too high... |
I've removed that too if that's what you're talking about. |
Before I test again: can the author confirm this is fixed now? Meteors happening at a reasonable rate? |
Honestly, no. Try testing it again and if there's a reasonable amount of meteors, I guess it was just a fluke before. If it happens again, I'll try actually looking into it. |
|
But have you tested these changes yourself or not? |
Is doing it with freebuild so that it doesn't take forever good enough? I don't see how it would change things. |
FYI, in a normal game you can press P to immediately unlock the editor to go to the next generation as long as you have cheats enabled. That's how I tested this PR previously. |
|
I'm back from testing (why did auto-evo take so long?!) and in 10 generations there were 3 meteor impact events. This seems exactly right considering the chance is 0.28. Fun fact, two of them were on the first and last generations! |
|
Also, better way is to use auto-evo tool ;) |
Accidental-Explorer
left a comment
Accidental-Explorer
left a comment
There was a problem hiding this comment.
With the compound buff removed, I think the impact of this change is relatively small. But I have confirmed there are no problems caused, and meteors are occurring at a reasonable rate.
hhyyrylainen
left a comment
hhyyrylainen
left a comment
There was a problem hiding this comment.
The code change looks fine to me know and I tested in-game and meteors seem to be triggering pretty normally with these changes.
I re-requested review from @Patryk26g in case there's some aspect of the code I missed.
|
Looks good for me now! |
5 participants
Brief Description of What This PR Does
This PR buffs the meteor impact event by adding a new rare impact size that affects all surface patches in the regions adjacent to the selected patch.
Related Issues
This partially implements the environmental events part of this thread.
Hopefully I can do more of that thread in the near future.
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.