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.

Sign up for GitHub

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

Jump to bottom

Add battleconf for party leave behavior #2386

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add battleconf for party leave behavior #2386

wants to merge 1 commit into from

Conversation

bWolfie
Copy link
Contributor

@bWolfie bWolfie commented Feb 25, 2019

Pull Request Prelude

Changes Proposed

Note: Was unsure of the best way to go about this.

Some time ago the behavior was changed for when a party leader leaves the party. I feel a config option should have been added to retain the old behavior.

Old: Disband the party.
Current: Pass the party leader to another player.
New: Uncomment #define PARTY_LEADER_LEAVE in src/config/classes/general.h

Perhaps others can input on a better way to implement a config option.

Issues addressed:

N/A

@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

@AnnieRuru
Copy link
Contributor

imo, should move this config into conf folder

although our party_share_level is define inside conf/common/inter-server.conf

4144
4144 requested changes Feb 25, 2019
View reviewed changes
src/config/classes/general.h Outdated
* when enabled, parties will be disbanded if its leader leaves the party
* uncomment to enable
**/
//#define PARTY_LEADER_LEAVE
Copy link
Contributor

Choose a reason for hiding this comment

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

better add configuration option for this

@AnnieRuru
Copy link
Contributor

AnnieRuru commented Feb 25, 2019 •
edited
Loading

actually I'm thinking, why not just add an atcommand for it ?
we already have @party @changeleader @partyoption ...
so just add @partydisband something like that ...

@bWolfie
Copy link
Contributor Author

bWolfie commented Feb 26, 2019

actually I'm thinking, why not just add an atcommand for it ?
we already have @party @changeleader @partyoption ...
so just add @partydisband something like that ...

Yeah that sounds better.

@Emistry
Copy link
Member

Emistry commented Feb 26, 2019

@partydisband doesn't sound like it solve the issue.
It create confusion with the party disband/leaving behaviour.
it could end up @partydisband and /leave work differently: Imagine you end up with such behaviour

@partydisband - leave party and remove the party (originally should behave like /leave)
/leave - leave party (and currently it would assign leader to another party member)

not to mention if the party manipulation script commands are planned to be added in future, it create unnecessary workaround to handling both behaviour simultaneously.

beside if player want to change/switch party leader, they could just use the @changeleader atcommand ?

So, adding a configuration in battle/conf/party.conf would most likely a more feasible option here, to switch the party disband behaviour.

@bWolfie bWolfie changed the title Add config for party leave behavior Add battleconf for party leave behavior Feb 26, 2019
@bWolfie
Copy link
Contributor Author

bWolfie commented Feb 26, 2019 •
edited
Loading

I changed it to a battleconf option. I don't really understand it all. Let me know what you think.

@bWolfie bWolfie force-pushed the party_leave_behavior branch 5 times, most recently from 2398803 to 6c1caa1 Compare February 26, 2019 09:15
@AnnieRuru AnnieRuru added type:enhancement Issue describes an enhancement or feature that should be implemented component:core Affecting the Hercules core (i.e. not the game mechanics directly) codereview:needsedits Some edits have been requested before the pull request can be accepted labels Feb 26, 2019
AnnieRuru
AnnieRuru requested changes Feb 26, 2019
View reviewed changes
src/char/int_party.c
for (j = 0; j < MAX_PARTY; j++) {
if (!p->party.member[j].account_id)
continue;
mapif->party_withdraw(party_id, p->party.member[j].account_id, p->party.member[j].char_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if the party member is offline ?
isn't that ... this configuration should just disband the party ?
you still kick them one by one though ... and expect them all to be online ...

tested

this modification doesn't check offline party members
if I have member A leave party while member B is offline, it shows a message for disband the party
but in-fact, the party leader pass to member B

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats why i said i dont understand it completely. need help. can you take over PR?

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

@AnnieRuru AnnieRuru AnnieRuru requested changes

@4144 4144 4144 requested changes

Assignees
No one assigned
Labels
codereview:needsedits Some edits have been requested before the pull request can be accepted component:core Affecting the Hercules core (i.e. not the game mechanics directly) type:enhancement Issue describes an enhancement or feature that should be implemented
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bWolfie @HerculesWSAPI @AnnieRuru @Emistry @4144