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

Various engine logic fixes #1439

Merged
merged 15 commits into from Nov 1, 2018

Conversation

Projects
None yet
4 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Oct 12, 2018

These are RPT_RT compatibility fixes.

Fixes some of the issues in #1418

@fmatthew5876 fmatthew5876 changed the title Fix skill usability bug Various engine logic fixes Oct 12, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:fix_skill_usable branch 3 times, most recently from 40804da to 47f0c28 Oct 12, 2018

@carstene1ns carstene1ns requested a review from Ghabry Oct 12, 2018

@Ghabry Ghabry referenced this pull request Oct 14, 2018

Closed

Event Commands fixes #1401

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:fix_skill_usable branch from 62ce9bd to b0c5d7b Oct 17, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 17, 2018

I've tested with rm2k RPG_RT.exe and confirmed that battles count doesn't increment until the battle ends.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:fix_skill_usable branch from c678ffa to 686214e Oct 27, 2018

@EasyRPG EasyRPG deleted a comment from fdelapena Oct 28, 2018


inline void Game_Party::IncRunCount() {
++data().escapes;
}

This comment has been minimized.

@Ghabry

Ghabry Oct 28, 2018

Member

Educational question:
I wonder about this inline void getter/setter. Are there any C++ references that suggest this? Is there an advantage in putting this in the header instead of cpp file?

This comment has been minimized.

@fmatthew5876

fmatthew5876 Oct 28, 2018

Author Contributor

Inlining is why C++ is so fast and yet enables high level abstractions. Functions which are very small should be inlined for performance. These Game_Party methods either just return or increment an integer. There is no benefit to paying the overhead of an entire function call for this.

The real power of this is when things are combined together because it enables the compiler optimizer to do smart things. For example, imagine you had a function foo(int p) and the first thing it does is check if p == 0. Imagine your calling code sets int p = 3 and then calls foo(p). If foo() is inlined, the compiler can see that p can never be zero, and therefore it could remove the entire if (p == 0) { ... } block from the code.

A good example is unique_ptr. All of unique_ptr is inlined, so in most cases if you write a piece of code using unique_ptr the compiler will cleanup the memory for you but when the optimizer hits you code the unique_ptr gets completely removed and it's as if you wrote new and delete by hand in all the right places.

You can play around with this stuff on godbolt: https://gcc.godbolt.org/

return Utils::ReplacePlaceholders(
message,
{'S', 'V', 'U'},
{GetTarget()->GetName(), ss.str(), points}
{GetTarget()->GetName(), std::to_string(value), points}

This comment has been minimized.

@Ghabry

Ghabry Oct 29, 2018

Member

We had problems with missing std::to_string on some platforms years ago (we have Utils::ToString as an alternative) but I will recheck if this is still problematic.

Show resolved Hide resolved src/game_map.h
@@ -176,6 +176,9 @@ void Game_Interpreter::Update() {
if (Main_Data::game_player->IsBoardingOrUnboarding())
break;

if (Main_Data::game_player->InVehicle() && Main_Data::game_player->GetVehicle()->IsAscendingOrDescending())
break;

This comment has been minimized.

@Ghabry

Ghabry Oct 29, 2018

Member

(Just a note) Another timing issue with vehicles, we had an almost similiar one in Witch's Heart before: 9659bae

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:fix_skill_usable branch from 686214e to 2c833ab Oct 29, 2018

@Ghabry

Ghabry approved these changes Oct 29, 2018

Copy link
Member

Ghabry left a comment

The std::to_string issues we had before appear to be solved (currently still building 3ds and Wii but I assume they will work as they are all using devkitPro stuff)

@fdelapena fdelapena removed the Needs Rebase label Oct 29, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:fix_skill_usable branch from 2c833ab to 53f00a7 Oct 31, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Oct 31, 2018

I simplified "Command OpenMenuScreen should not play SFX " to not introduce another Game_Temp global variable.

fmatthew5876 added some commits Oct 12, 2018

Fix skill usability bug
Skills which affect states which end after battle should not
be usable in the menu.
Command OpenMenuScreen should not play SFX
Only play Decision SFX when menu is opened with the
escape key.

fmatthew5876 added some commits Oct 12, 2018

Scene_Debug: Left/Right use IsRepeated()
Makes it easier to scroll through lots of switches
and compatible with RPG_RT.
Interpreter: Wait for airship to ascend/descend
Fixes bugs in events with code like:
```
GetOnOffVehicle
Teleport to Map
```

In Player the teleport would not wait for the airship landing to finish,
and so you'd land on the wrong map.
Refactor Game_Party
- add incrment methods
- inline battle methods
- refactor and inline data() accessor
- start using const
Increment battle counts on battle end.
Also resets battle_result to abort on startup, so this becomes
the default if the battle exits.
Fixes for Ctrl (DEBUG_THROUGH)
- Ctrl lets you walk left of 0 and right of map width into infinity.
  This patch doesn't let you off the edge of the map.
- Ctrl doesn't work for vehicles in RPG_RT but does in Player. The
  behavior has been changed to be compatible.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:fix_skill_usable branch from 53f00a7 to ae12a82 Nov 1, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Nov 1, 2018

rebased to master

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Nov 1, 2018

The std::to_string issues we had before appear to be solved

Seems we should get rid of this in utils then.

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Nov 1, 2018

Seems we should get rid of this in utils then.

Added a commit to remove Utils::ToString

@carstene1ns carstene1ns merged commit 937401d into EasyRPG:master Nov 1, 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.