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

Disable HP bar on Emperium and MVP by default #2821

Merged
merged 1 commit into from Dec 15, 2020

Conversation

Samuel23
Copy link
Contributor

Pull Request Prelude

Changes Proposed

Disable HP bar on Emperium and MVP by default
Added configuration via battle conf to enable again

Issues addressed:
#744 (comment)

@4144
Copy link
Contributor

4144 commented Aug 25, 2020

see compilation errors in travis

src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
Copy link
Member

@Emistry Emistry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps consider something like these?

p.maxHP = -1;
p.HP = -1;
if (status_get_hp(bl) < status_get_max_hp(bl)) {
	if ((battle_config.show_monster_hp_bar == 1 && !(md->class_ == MOBID_EMPELIUM || md->status.mode & MD_BOSS))
		|| (battle_config.show_monster_hp_bar & 2 && md->status.mode & MD_BOSS)
		|| (battle_config.show_monster_hp_bar & 4 && md->class_ == MOBID_EMPELIUM)
	) {
		p.maxHP = status_get_max_hp(bl);
		p.HP = status_get_hp(bl);
	}
}

less redundant? easier to read?

or even better create a new method

static bool is_show_monster_hp_bar(struct mob_data *md, struct block_list *bl) {
	if (status_get_hp(bl) < status_get_max_hp(bl)) {
		if ((battle_config.show_monster_hp_bar == 1 && !(md->class_ == MOBID_EMPELIUM || md->status.mode & MD_BOSS))
			|| (battle_config.show_monster_hp_bar & 2 && md->status.mode & MD_BOSS)
			|| (battle_config.show_monster_hp_bar & 4 && md->class_ == MOBID_EMPELIUM)
		) {
			return true;
		}
	}
	return false;
}

so the code can reuse it at other places? since its applicable to more than 1 places.

@Samuel23
Copy link
Contributor Author

@Emistry edited as a function just like you suggested. Please see changes, thank you! :)

src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.c Outdated Show resolved Hide resolved
src/map/clif.h Outdated Show resolved Hide resolved
@4144 4144 changed the base branch from stable to master September 22, 2020 15:16
@4144
Copy link
Contributor

4144 commented Sep 22, 2020

ops, i try to fix conflicts with github interface, and it created merge commit.
please squash all into one commit, merge commit you can remove.

@Samuel23
Copy link
Contributor Author

ops, i try to fix conflicts with github interface, and it created merge commit.
please squash all into one commit, merge commit you can remove.

@4144 so how do i do this? :D

@4144
Copy link
Contributor

4144 commented Sep 24, 2020

@4144 so how do i do this? :D

I also found this
static bool clif_show_monster_hp_bar(const struct mob_data *md, struct block_list *bl) {
should be

static bool clif_show_monster_hp_bar(const struct mob_data *md, struct block_list *bl)
{

About rebase.
At first need remove merge commit, if it present locally in your repo already git reset --hard HEAD~ (remove last commit)
if here no merge commit, can do other steps.
do interactive rebase and squash commits into one. git rebase -i 0be29a61d5fa53bbfbc566cd45bed83f23125e11
In editor mark all your commits except first with fixup, save.
As result you should get one commit with all your changes.
Then run git push -f

@4144
Copy link
Contributor

4144 commented Sep 24, 2020

also need rebase and fix conflict. about this better read somewhere. i not sure what tools you using and how better explain it

@AnnieRuru
Copy link
Contributor

AnnieRuru commented Oct 4, 2020

#2008

can do the way aegis do ?
define all monsters into class ...
means add a new field in mob_db.conf

define all MVP as Class_MVP
define all the monster boss as Class_Boss
define all the guardians/emperium as Class_Guardian
the rest default to Class_Normal

then the battle_config has 4 fields

// Show Monster HP Bar (Note 3)
// 0x1 = (Class_Normal) Show hp bar on normal monsters
// 0x2 = (Class_MVP) Enable hp bar on MVP
// 0x4 = (Class_Boss) Enable hp bar on boss monsters
// 0x8 = (Class_Guardian) Enable hp bar on Emperium/Guardians/Barricades
show_monster_hp_bar: 1

barricade also do not show HP bar, it is Class_Guardian ? or Class_Boss ?
I think is Class_Guardian ... I guess ... @Asheraf ?

btw I think this is the actual and correct fix,
this PR looks custom when Asheraf mention Class_Normal/Class_Guardian blah blah

EDIT: rathena has this

enum e_classAE : int8{
	CLASS_NONE = -1, //don't give us bonus
	CLASS_NORMAL = 0,
	CLASS_BOSS,
	CLASS_GUARDIAN,
	CLASS_BATTLEFIELD,
	CLASS_ALL,
	CLASS_MAX //auto upd enum for array len
};

CLASS_BATTLEFIELD ??

@Kenpachi2k13
Copy link
Member

#2008

can do the way aegis do ?
define all monsters into class ...
means add a new field in mob_db.conf

define all MVP as Class_MVP
define all the monster boss as Class_Boss
define all the guardians/emperium as Class_Guardian
the rest default to Class_Normal

In mob.h we have this enumeration which is used in the monster spawn code:

enum e_bosstype {
	BTYPE_NONE = 0,
	BTYPE_BOSS = 1,
	BTYPE_MVP = 2,
};
if (strcmp(w2, "boss_monster") == 0)
	mobspawn.state.boss = BTYPE_MVP;
else if (strcmp(w2, "miniboss_monster") == 0)
	mobspawn.state.boss = BTYPE_BOSS;
else
	mobspawn.state.boss = BTYPE_NONE;

Added configuration via battle conf to enable again

Redesigned showhpbar as a clif function
Thanks to @Emistry for the information

Signed-off-by: Samuel23 <johnsamuel_santos3@yahoo.com>
@MishimaHaruna MishimaHaruna merged commit dcac169 into HerculesWS:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants