Skip to content

Conversation

@carloshenrq
Copy link
Contributor

Pull Request Prelude

Changes Proposed

This pull request, add 2 new mapflags by default. nostorage and nogstorage.
This change only affect atcommands like @storage and @gstorage, NPCs are still able to open player storage.

By now, we can apply this using plugins, but i believe this flag can be default in hercules.

Affected Branches:

master

Issues addressed:

Known Issues and TODO List

@HerculesWSAPI
Copy link
Contributor

This change is Reviewable

@4144
Copy link
Contributor

4144 commented Sep 20, 2018

please squash all commits, because fix commits is useless until not pushed to master

@carloshenrq
Copy link
Contributor Author

@4144 Thank you for explain and be clear. I'm new in pull requests... Anyway, i've combined them in one commit.

@carloshenrq
Copy link
Contributor Author

@4144 Hi! Can you review this? Thank you =]

@4144
Copy link
Contributor

4144 commented Nov 27, 2018

@carloshenrq probably same flags need check also in script commands related to storages?

@carloshenrq
Copy link
Contributor Author

carloshenrq commented Nov 27, 2018

@4144 Actually I don't think it should be needed. There're some servers that set @storage for VIP players... but inside battlegrounds, for example, this might be blocked to prevent abuse... Anyway, I can put a type in the mapflag... like:

prontera mapflag nostorage 1 // block only atcommands
prontera mapflag nostorage 2 // block only script commands
prontera mapflag nostorage 3 // block atcommands and script commands

but the block de script commands'll be last used.

@Asheraf
Copy link
Contributor

Asheraf commented Nov 27, 2018

Isn't this doable via the map zone db, and it's much more configurable since you can set any command not just the storage and you can specify a group id to bypass the restriction.. other than that if you're still adding this i would suggest renaming it to noatstorage noatgstorage since that is a more accurate naming.

@4144
Copy link
Contributor

4144 commented Nov 27, 2018

yes probably type can be good idea

@carloshenrq
Copy link
Contributor Author

@4144 i've added the types and as @Asheraf says, added a group permission to bypass the flag. Can't test it now, but it's done.

@carloshenrq
Copy link
Contributor Author

@4144 Ok! I'll change =]

Just for record... I've copied and pasted from others mapflags like this one: (refer to if( !state )) and %s", params)

} else if ( !strcmpi(flag,"long_damage_rate") ) {
	if( !state ) {
		if( map->list[m].long_damage_rate != 100 ) {
			sprintf(rflag,"long_damage_rate\t%d",map->list[m].long_damage_rate);
			map_zone_mf_cache_add(m,rflag);
		}
	} if( sscanf(params, "%d", &state) == 1 ) {
		if( state != map->list[m].long_damage_rate ) {
			sprintf(rflag,"long_damage_rate\t%s",params);
			map_zone_mf_cache_add(m,rflag);
		}
	}
} else if (!strcmpi(flag,"nocashshop")) {

Should be an issue/task for change others to new format?

@4144
Copy link
Contributor

4144 commented Nov 27, 2018

yes in code many wrong style. but in new code better not add more style issues.

about task yes, probably some one should fix style issues

@carloshenrq
Copy link
Contributor Author

@4144 Have done all changes! Can you review it? Thank you =]

@carloshenrq
Copy link
Contributor Author

carloshenrq commented Nov 29, 2018

@dastgirp and @4144 All changes are done.

nostorage 1 -- blocks only @storage
nostorage 2 -- blocks only openstorage();
nostorage 3 -- blocks @storage and openstorage()

nogstorage 1 -- blocks only @gstorage
nogstorage 2 -- blocks only guildopenstorage();
nogstorage 3 -- blocks @gstorage and guildopenstorage()
@carloshenrq
Copy link
Contributor Author

carloshenrq commented Dec 4, 2018

@4144 and @dastgirp can you guys review it again? Thank you =]

@MishimaHaruna MishimaHaruna added this to the Release v2019.06.02 milestone Jun 1, 2019
@MishimaHaruna MishimaHaruna merged commit eb0faf2 into HerculesWS:master Jun 2, 2019
@carloshenrq carloshenrq deleted the nostorage branch June 5, 2019 11:48
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.

6 participants