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

More battle fixes #905

Merged
merged 13 commits into from Jun 27, 2016

Conversation

Projects
None yet
5 participants
@Ghabry
Member

Ghabry commented Jun 4, 2016

To explain the not so obvious Things:
Due to changes to make the z-value working correctly only the last anim Frame was rendered. The BattleAnimation inherits from Sprite now which allows direct drawing on the Screen in that for-loop.
Seems to work fine only had to rename some conflicting functions.

The RNG Change:
I had a skill with 50% hit rate that basicly almost missed. I read on stackoverflow that rand with modulo can result in a bad Distribution. Therefore I switched this to the modern C++11 Api.

@@ -284,3 +289,12 @@ bool Utils::IsBigEndian() {
return(d.c[0] == 1);
}
int32_t Utils::GetRandomNumber(int32_t from, int32_t to) {
std::uniform_int_distribution<int32_t> dist(from, to);

This comment has been minimized.

@Zegeri

Zegeri Jun 5, 2016

Member

Using int32_t as the numeric type might be undefined behavior. From the documentation:

The effect is undefined if this is not one of short, int, long, long long, unsigned short, unsigned int, unsigned long, or unsigned long long.

See also this.

Edit: Never mind, int32_t must be a typedef of one of those types.

@Zegeri

Zegeri Jun 5, 2016

Member

Using int32_t as the numeric type might be undefined behavior. From the documentation:

The effect is undefined if this is not one of short, int, long, long long, unsigned short, unsigned int, unsigned long, or unsigned long long.

See also this.

Edit: Never mind, int32_t must be a typedef of one of those types.

@Zegeri

This comment has been minimized.

Show comment
Hide comment
@Zegeri

Zegeri Jun 5, 2016

Member

By the way, could you change this line? It should be:

this->agility = effect;
Member

Zegeri commented Jun 5, 2016

By the way, could you change this line? It should be:

this->agility = effect;

@Ghabry Ghabry referenced this pull request Jun 14, 2016

Open

Battle System: Remaining, not implemented stuff #821

21 of 35 tasks complete
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jun 17, 2016

Member

Ara Fell: Attacking all enemies with a multi target skill and healing all actors.

animation

UiD: Start animation and multi target animation

animation2

Member

Ghabry commented Jun 17, 2016

Ara Fell: Attacking all enemies with a multi target skill and healing all actors.

animation

UiD: Start animation and multi target animation

animation2

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jun 20, 2016

Member

This needs a retest of that Hentai Film game #906 because of the Y-BattleAnimation offset change.
At least Ara Fell needs the "/ 3" but maybe it depends on another editor setting if it's / 3 or / 2.

Member

Ghabry commented Jun 20, 2016

This needs a retest of that Hentai Film game #906 because of the Y-BattleAnimation offset change.
At least Ara Fell needs the "/ 3" but maybe it depends on another editor setting if it's / 3 or / 2.

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Jun 20, 2016

Member

Note: You changed BattleAnimationBattlers::Draw(), while Chris changed BattleAnimationChara::Draw().

Member

carstene1ns commented Jun 20, 2016

Note: You changed BattleAnimationBattlers::Draw(), while Chris changed BattleAnimationChara::Draw().

Show outdated Hide outdated src/battle_animation.cpp
void BattleAnimation::OnBattleSpriteReady(FileRequestResult* result) {
if (result->success) {
sprite.reset(new Sprite());
//Normally only battle2 sprites are "large" sprites - but the check doesn't hurt.

This comment has been minimized.

@ChristianBreitwieser

ChristianBreitwieser Jun 20, 2016

Member

The changes from the fix for #906 are undone in this file.

@ChristianBreitwieser

ChristianBreitwieser Jun 20, 2016

Member

The changes from the fix for #906 are undone in this file.

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Jun 20, 2016

Member

Yep, needs a rebase.

EDIT: Needs a better rebase 🐎.

Member

carstene1ns commented Jun 20, 2016

Yep, needs a rebase.

EDIT: Needs a better rebase 🐎.

Show outdated Hide outdated src/battle_animation.cpp
//If animation is targeted on the screen
if (animation.scope == RPG::Animation::Scope_screen) {
DrawAt(SCREEN_TARGET_WIDTH / 2, SCREEN_TARGET_HEIGHT / 2);
return;

This comment has been minimized.

@ChristianBreitwieser

ChristianBreitwieser Jun 21, 2016

Member

Sorry, but the code which fixes an animation targeting the screen on a map is still missing ;)

@ChristianBreitwieser

ChristianBreitwieser Jun 21, 2016

Member

Sorry, but the code which fixes an animation targeting the screen on a map is still missing ;)

This comment has been minimized.

@Ghabry

Ghabry Jun 21, 2016

Member

This is what happens when you use a diff tool you are not familiar with while rebasing >.<

@Ghabry

Ghabry Jun 21, 2016

Member

This is what happens when you use a diff tool you are not familiar with while rebasing >.<

@fdelapena fdelapena merged commit 15c8ffe into EasyRPG:master Jun 27, 2016

6 checks passed

Android Build finished.
Details
Linux Build finished.
Details
OSX Build finished.
Details
Windows Build finished.
Details
Windows-x64 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