Conversation
|
Hi @DJ-Meyers if you have some spare time could you review this PR? It's about encounters and you recently merged a similar PR. |
|
Hi @jemarq04, now that I fully trust you on anything related to encounters, I might take too much advantage of your kindness. If you could help us here again with this PR, we would appreciate it. Let me know if I'm pinging you too often! |
No worries, I enjoy helping with the project! I'll take a look. |
| 43,5,Apparition rare dans l'eau avec un parfum ou un combo capture | ||
| 43,9,Rare spawn in water with lure or catch combo | ||
| 43,5,Apparition rare dans l'eau avec un parfum ou un combo capture No newline at end of file | ||
| 44,9,"Walking in grass, flowers, terrain, or in a cave" |
There was a problem hiding this comment.
I wonder if we can come up with something more descriptive here that make this unique to horde encounters, as this is a bit general. Maybe something like "Horde encounter within grass, flowers, terrain, or in a cave".
With that said, what is meant by "terrain" here?
There was a problem hiding this comment.
How about "Encountering 5 Pokemon within grass, flowers, on terrain, or within a cave"?
Terrain was a catch-all for cases like the Sandshrew in Sand (ORAS). I don't recall if you can encounter hordes in the swamp tiles in X and Y, but for now, we take out the mention of terrain.
There was a problem hiding this comment.
I'll confirm that you can't get hordes in swamps in X/Y. I was doing rng manips there specifically to avoid hordes.
jemarq04
left a comment
There was a problem hiding this comment.
I've left a couple comments to review above, once those are settled I'll do a more in-depth check of the encounters themselves with a helper script I used to do this previously.
Updated Encounter Method Id to match Horde Encounter Id
|
Small issue is this pr conflicts with mine about the event data. We're both currently using encounter method 44. Is this something that can be fixed on merge, or should I update pr 1417 to accommodate for this? And was this something I should've looked at in advance and taken into account when I made the PR initially? |
This usually gets handled when dealing with merge conflicts I believe. In this case, it would be easier to refactor this encounter method to the one after yours and update accordingly. Maybe a maintainer can comment, but from what I've seen it usually just gets handled when things get final approval and need to be updated pre-merge |
@jemarq04 It looks like the issue extends to encounters and encounter_slots as well. |
Ah, you're right I overlooked that. I need some time to look over this PR, so you can always update things to work alongside @notblisy's PR in the meantime. Sorry about the inconvenience on that, I think it's the unfortunate consequence of both of you submitting encounter PRs at the same time by coincidence |
|
Fair point, it doesn’t matter who modifies their PR as long as you can both agree. Both options are fine, I’ll be taking a look at the encounter data in this PR tonight |
Adding missing Horde Encounter information for XY and ORAS. The information was copied from Bulbapedia's page about the mechanic. Also noticed some minor differences (level max for lombre, and an area labeled "unknown" that can be set as "outside" to match what Bulbapedia refers to it as.)
Some considerations for the future are the differences in encounter between RSE and ORAS are not correctly documented (see Shoal Cave), and some Horde Encounters having a 4-1 split in certain encounters (not sure how to accomplish this with current data structure).
(also sorry for not merging latest master into my branch last time. 😣)