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

Random Encounters fixes #1455

Merged
merged 4 commits into from Nov 18, 2018

Conversation

Projects
None yet
5 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Oct 16, 2018

Depends on: #1441

Info from here: EasyRPG/liblcf#261

  • Correctly load encounter_rate from saved game
  • Compute first strike chance on random encounter.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch 2 times, most recently from 8bd330e to 0afc9a9 Oct 16, 2018

@fmatthew5876 fmatthew5876 changed the title Correctly handle map encounter_steps from loaded savegame Random Encounters fixes Oct 17, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch from 0afc9a9 to 403950d Oct 17, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 17, 2018

@CherryDT

I wanted to ask you some questions to clarify the behavior with Random Encounters. Hopefully we can fix everything encounter related and get 100% compatibility in this PR.

  1. SavePartyLocation 0x7C encounter steps

Is this value saved and loaded exactly? If I save my game with 5 steps left will I always hit a random encounter on exactly 5 steps when I load the game? Are there any special changes made to this value on loading the game?

  1. What is the algorithm used for recomputing location encounter_steps in rm2k?

Player does this:

a = rand(0, rate -1)
b = rand(0, rate -1)
steps = (a + b + 1) * 100

And then at each step:

steps -= terrain(player_x, player_y).encounter_rate
if (steps <= 0) {
   doEncounter();
}

Is that accurate?

I remember in old versions of rm2k you could get into an encounter in a single step if you were unlucky. I think they fixed that in newer versions. It looks like that same thing could happen here.

  1. Is location steps always recomputed from scratch when you change maps?

  2. What happens when the interpreter processes a ChangeEncounterRate command?

Does location steps get immediately recomputed against the new rate or is it left alone until after the next battle? Is the custom encounter rate from this event command always reset when you change maps?

  1. Any other wierd edge cases I should know about not covered here?

Thanks!

@CherryDT

This comment has been minimized.

Copy link

CherryDT commented Oct 18, 2018

On every step, the SavePartyLocation.encounter_steps is increased by Terrain.encounter_rate.

Teleporting the party (also on the same map) or an encounter resets it to zero. This value is saved and loaded to/from save files normally like any other value.

There is then a probability calculation which is not linear but in steps, and it's based on the ratio of SavePartyLocation.encounter_steps and SaveMapInfo.encounter_rate:

2k3 and modern 2k

SavePartyLocation.encounter_steps/SaveMapInfo.encounter_rate ratio threshold Probability modifier
0+ 1/16
20+ 1/8
40+ 1/4
60+ 1/2
100+ 2
140+ 4
160+ 8
180+ 16

Old 2k (I think before Value)

SavePartyLocation.encounter_steps/SaveMapInfo.encounter_rate ratio threshold Probability modifier
0+ 1/2
20+ 1/1.5
50+ 1/1.2
100+ 1.2
200+ 1.5

(In my opinion, it's weird that there is never a modifier of 1. Something doesn't make sense there even in the "modern" calculation!)

Then, the probability to trigger an encounter is calculated using:

Probability = (1 / SaveMapInfo.encounter_rate) * Probability modifier * (Terrain.encounter_rate / 100)

I'm not sure yet why the multiplication with the terrain encounter_rate is done at the end again though. EDIT: Actually I see why, it's to give terrains immediate effect on top of the accumulative effect.

Also, RPG_RT does at the end if(RandomInteger(0 to X-1) < Probability*X), and old 2k has less granularity here with an X of 1000, while modern 2k and 2k3 uses an X of 65536.

Oh, and regarding the question with ChangeEncounterRate: It simply changes SaveMapInfo.encounter_rate so during the next step, the new value is used in the calculation.

By the way: a = rand(0, x); b = rand(0, x); result = a + b; is an anti-pattern because believing that multiple random values combined make the result "more random" is a fallacy most of the time - you could just use result = rand(0, x*2);.

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 19, 2018

I've added logic to reset the encounter rate when you teleport to the same map.

I tested this by putting a breakpoint on Game_Map::ResetEncounterSteps() and verifying it was called when it is supposed to be.

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 19, 2018

@CherryDT

If you board a vehicle, does it affect the encounter rate? What about the airship? Does encounter_steps temporarily freeze while flying or does it get reset to 0 when you land? Does it reset for boat or ship?

I am going to assume that when you press Ctrl in RPG_RT it just freezes the encounter system until you release the key.

What happens when the encounter_rate == 0. Does encounter_steps always just get locked to 0 or does it continue ticking upward to infinity? ChangeEncounterRate doesn't reset the encounter_steps, so what happens if you ChangeEncounterRate to or from 0?

Do they do this entire calculation using only integer math and rational numbers?

EDIT: I pushed a new implementation of this algorithm using floating point.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch 2 times, most recently from aa1356f to 141e106 Oct 19, 2018

@CherryDT

This comment has been minimized.

Copy link

CherryDT commented Oct 19, 2018

To answer your questions:

  • Ground vehicles don't affect the calculation.
  • In the following cases, the calculation is not done ("freezing" the counter effectively): Ctrl pressed, map encounter rate is zero, inside airship
  • An encounter resets the counter to zero.
  • The algorithm is using floating point.

Also, a detail I realized: the encounter waiting flag is checked before this calculation. So if an encounter is scheduled, it would set this flag and then only next frame initiate the actual encounter

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch from 141e106 to 35c6e9c Oct 19, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 19, 2018

Thanks for the clarifying info and the code review. I fixed the typos in the table.

Also, a detail I realized: the encounter waiting flag is checked before this calculation. So if an encounter is scheduled, it would set this flag and then only next frame initiate the actual encounter

I'm guessing you refer to SavePartyLocation::encounter_calling (0x7D) flag here?

This one might be important to get right so encounters behave correctly w.r.t. to running events.

@fdelapena fdelapena added the Battle label Oct 28, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch from 35c6e9c to 7784662 Nov 3, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Nov 3, 2018

Rebased to master

@@ -918,25 +935,61 @@ void Game_Map::UpdateEncounterSteps() {
return;
}

location.encounter_steps = location.encounter_steps - terrain->encounter_rate;

This comment has been minimized.

@Ghabry

Ghabry Nov 3, 2018

Member

the reason this was written in such an ugly way (not using -=) was btw for support in my automagical refactor liblcf field-access to Get/Set function converter

int throw_two = Utils::GetRandomNumber(0, rate - 1);
const auto pmod = row.pmod;
const auto p = (1.0f / float(encounter_rate)) * pmod * (float(terrain->encounter_rate) / 100.0f);
auto draw = std::uniform_real_distribution<float>()(Utils::GetRNG());

This comment has been minimized.

@Ghabry

Ghabry Nov 3, 2018

Member

is std::uniform_real_distribution implementation standardized? I mean does it give the same values for the same seed across all C++ implementations (clang c++ lib, gcc c++ lib, MSVC)?

This comment has been minimized.

@fmatthew5876

fmatthew5876 Nov 3, 2018

Author Contributor

Good question. I can't find anything that guarantees this.

uniform_real_distribution does not actually play a part in drawing random numbers. The rng stored in Utils::GetRNG() does that. The distribution is just a deterministic mathematical function which takes an input random number in some integer range and converts it to floating point. I guess standard library implementations could use different combinations of floating point expressions which could result is slightly different answers.

That being said, this is the right way to do this. Modulus and all the other tricks we used in the past with C rand() are bad. They don't actually give you good random numbers.

https://en.cppreference.com/w/cpp/numeric/random/uniform_real_distribution

This comment has been minimized.

@fmatthew5876

fmatthew5876 Nov 3, 2018

Author Contributor

Also, Here is a great cppcon talk on the C++11 random facilities. Worth the time to watch.

https://www.youtube.com/watch?v=6DPkyvkMkk8

This comment has been minimized.

@Ghabry

Ghabry Nov 3, 2018

Member

The problem with uniform_int_distribution was discussed here: #1104
But we don't provide a way to get random float numbers through Utils, so I can't suggest an alternative. Should be fixed before 0.6 as it breaks input-playbacks of input-recordings (see help/man page) when using different platforms.

This comment has been minimized.

@fmatthew5876

fmatthew5876 Nov 15, 2018

Author Contributor

I will change this to use PercentChance() when #1487 gets merged.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 3, 2018

code looks fine to me, for the algorithm I need to fill a spreadsheet first...

@Ghabry Ghabry added this to the 0.5.5 (or 0.6.0) -> we will see milestone Nov 4, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch from 7784662 to fdcda71 Nov 6, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Nov 6, 2018

Rebased onto master.

The one commit that handled save data encounter_steps was dropped as this is now done by the lsd fixes in master.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch from fdcda71 to 0d91ea1 Nov 6, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:save_steps branch from 0d91ea1 to 260fa13 Nov 6, 2018

@Ghabry Ghabry self-requested a review Nov 10, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 18, 2018

Is this ratio-threshold table really correct? I have a hard time to understand this.

Let's say the map has a encounter rate of 25, then the Party will need 500 steps to reach a ratio of 20.

Which means the algorithm is always stuck at the lowest value for an extremely long time and has a constant encounter probability of 0.000025 (assuming terrain rate is 1).

Am I reading the algorithm wrong?

@CherryDT

This comment has been minimized.

Copy link

CherryDT commented Nov 18, 2018

It sounds for me like you are off by a factor of 100 (and in total therefore 10,000 due to the multiplication at the end) - the terrain rate is in percent! So a typical value for it would be 100, not 1!

So it would take 5 steps (not 500) to reach a ratio of 20, and at that point the chance would be ((1/25)/8)*1=0.005

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 18, 2018

Well, that's the code I use for the analysis:

enc = matrix(c(
  0, 1/16,
  20, 0.125,
  40, 0.25,
  60, 0.5,
  100, 2.0,
  140, 4.0,
  160, 8.0,
  180, 16.0,
  1000000, 16.0
), ncol=2, byrow=TRUE)

player_steps = seq(from = 0, to = 5000, by = 100)
map_enc_steps = 25

ratio = player_steps / map_enc_steps

res = sapply(ratio, FUN = function(x) {
  idx = 1
  while (x > enc[idx+1,1]) {
    idx = idx + 1
  }
  return (idx)
})

pmod = enc[res, 2]
p = (1.0 / map_enc_steps) * pmod * (100.0 / 100.0)

plot(player_steps / 100, p)

rplot03

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 18, 2018

Oh, now I see in the editor what you mean... sec

@CherryDT

This comment has been minimized.

Copy link

CherryDT commented Nov 18, 2018

Which would be correct at first glance, except that the terrain rate of 1 (percent!) is a totally unrealistic value. 100 is the default.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 18, 2018

okay fixed. k, so the algorithm is just really bad, thanks for the confirmation xD.

@CherryDT

This comment has been minimized.

Copy link

CherryDT commented Nov 18, 2018

It's not that bad if you think about it. I think that stepping is weird but apart from that it does its job giving a perceptible average rate that matches the one configured in the map with an expected randomness, while reducing the probability of very short and very long interval outliners, factoring in effects of the terrain in both accumulated and immediate effects.

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Nov 18, 2018

The way it steps up into infinity is pretty clever in my opinion. You can get randomness when you load a saved game without resetting the encounter steps. The reset operation is just writing to zero which makes initialization simpler and localizes all of the algorithm logic into the step function.

The old version of this in Don Miguel's rm2k was horrible though. It would make you sometimes get into a 2nd battle in one step. I guess they just didn't tune it well.

@CherryDT

This comment has been minimized.

Copy link

CherryDT commented Nov 18, 2018

@Ghabry even the fixed graph is misleading because the player_steps isn't really steps, it's "accumulated terrain effects", with the effect value being in percent, so it's effectively still off by x100 on the x axis (either that or player_steps is a misnomer)

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 18, 2018

At least I understand now why some would prefer to rename it from steps to rate... >.>

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Nov 18, 2018

Yeah, nevermind. After fixing this it makes more sense. THe stepping is still strange imo, but it gives a reasonable result now.

@Ghabry

Ghabry approved these changes Nov 18, 2018

@Ghabry Ghabry merged commit 8c9a08f into EasyRPG:master Nov 18, 2018

6 checks passed

Android (armeabi-v7a) Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows (x64) Build finished.
Details
Windows (x86) Build finished.
Details
web Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.