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

Implement Maniacs Battle Common Events #3307

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

MakoInfused
Copy link
Contributor

When using maniacs patch and we have a "Battle Begin" trigger set in the editor like this:
image

In battle we now have it get triggered like this:
image

When we use a "Battle (Parallel)" trigger in the editor like this:
image

In battle it will result in a repeated common event getting triggered every 5 seconds (since that is what my wait time it set to in the above image) like this:
EasyRPG-ManiacsBattleCommonEvents

@Ghabry
Copy link
Member

Ghabry commented Nov 27, 2024

The way how you do parallel process updating looks completely different to how it is done on the map.

For the map the common events all have a parallel process directly attached to them:

Player/src/game_map.cpp

Lines 1283 to 1308 in d9dd9be

bool Game_Map::UpdateCommonEvents(MapUpdateAsyncContext& actx) {
int resume_ce = actx.GetParallelCommonEvent();
for (Game_CommonEvent& ev : common_events) {
bool resume_async = false;
if (resume_ce != 0) {
// If resuming, skip all until the event to resume from ..
if (ev.GetIndex() != resume_ce) {
continue;
} else {
resume_ce = 0;
resume_async = true;
}
}
auto aop = ev.Update(resume_async);
if (aop.IsActive()) {
// Suspend due to this event ..
actx = MapUpdateAsyncContext::FromCommonEvent(ev.GetIndex(), aop);
return false;
}
}
actx = {};
return true;
}

(ignore the async stuff for now)


Maybe add UpdateBattle and IsWaitingBattleBackgroundExecution to the common events and do it via these functions?


What needs to be tested:

On the map parallel processes can be suspended and they will continue where they stopped. Does Maniac Patch do this in battle? To test this run two parallel processes:

PP1 (has run condition S1 = ON):

ShowMessage A
Set Switch S2 = ON
Set Switch S1 = OFF
ShowMessage B

PP2 (has run condition S2 = ON):

ShowMessage C
Set Switch S1 = ON
Set Switch S2 = OFF
ShowMessage D

The expected message output is A, C, B. D never shown.

At least when battle pps are like map pps.


Also worth testing if they keep the progress across battles:

For this simply do something like

PP1:

ShowMessage A
Abort Battle
ShowMessage B

When it resets it will always show A. When it remembers the state it will show B when a 2nd battle starts.


Also parallel processes can run... in parallel.

So you can do stuff like this:

PP1:

ShowMessage A
Wait 0
ShowMessage C

PP2:

ShowMessage B
Wait 0
ShowMessage D

I think the output should be A, B, C, D.

@Ghabry Ghabry added this to the 0.8.1 milestone Nov 29, 2024
@MakoInfused
Copy link
Contributor Author

MakoInfused commented Nov 30, 2024

@Ghabry I tested all of the exact scenarios as you outlined them and here are the results!

On the map parallel processes can be suspended and they will continue where they stopped. Does Maniac Patch do this in battle?

No. It seems to work in a much more simple/diferent way.

Suspension Test:
Map PP expected output: A, C, B. D never shown.
Maniacs Battle PP actual output: A, B, C, D.
My Implementation output: A, B. C. D is never shown.
Seems I'll need to fix this is as the events aren't all running in parallel to match Maniacs.

Resume Test:
Map PP expected output: When it remembers the state it will show B when a 2nd battle starts.
Maniacs Battle PP actual output: It resets and never shows anything except A.
My Implementation output: Works the same as Maniacs.

Parallel Test:
Map PP expected output: I think the output should be A, B, C, D.
Maniacs Battle PP actual output: A, B, C, A, D, C (repeats from beginning)
My Implementation output: B, D, A, C (repeated from beginning)
Seems this doesn't work the same either and will also need to be changed to match Maniacs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants