dataviruset / sm-hosties

The ultimate jailbreak plugin for SourceMod
https://forums.alliedmods.net/forumdisplay.php?f=155
GNU General Public License v3.0
29 stars 27 forks source link

Move GameType enum to lastrequest.inc #63

Closed DrKittens closed 7 years ago

DrKittens commented 7 years ago

Hosties.inc appears to just be stocks for sm_hosties to work. Lastrequest.inc is where the natives are and is meant to be included with other plugins. Lastrequest.inc requires this enum to compile, if its in Hosties.inc then Hosties.inc needs to be included aswell which i believe is bad practice as it creates bloat.

Some plugins will actually fail to compile using a clone of the github master as they (correctly) do not include hosties.inc, and only include lastrequest.inc. The error given will be something along the lines of undefined symbol game_unknown.

data-bomb commented 7 years ago

Stock functions do not cause bloat. They are removed if they're not used. This is fairly well documented in the Pawn guides.

Not sure why plugins are just using lastrequest.inc when the examples we had used both. It wasn't correct to just include lastrequest.inc.

However, I can see why this would present a problem. The default value of Game_Unknown in SetThirdPerson was a recent addition and this would break compatibility with a plugin incorrectly including only lastrequest.inc.

If anything, the change here should be to include hosties from lastrequest.inc so the error and direction is more clearly presented.

Sent from my iPhone

On Nov 15, 2016, at 10:34, DrKittens [Masked] wrote:

Preview: Hosties.inc appears to just be stocks for sm_hosties to work. This email is forwarded from a MASKED EMAIL you created using Blur. IF THIS IS SPAM, CLICK HERE TO BLOCK.

Want to shop safely and privately online? Get Blur Premium.

Hosties.inc appears to just be stocks for sm_hosties to work. Lastrequest.inc is where the natives are and is meant to be included with other plugins. Lastrequest.inc requires this enum to compile, if its in Hosties.inc then Hosties.inc needs to be included aswell which i believe is bad practice as it creates bloat.

Some plugins will actually fail to compile using a clone of the github master as they (correctly) do not include hosties.inc, and only include lastrequest.inc. The error given will be something along the lines of undefined symbol game_unknown.

You can view, comment on, or merge this pull request online at:

https://github.com/dataviruset/sm-hosties/pull/63

Commit Summary

Move GameType enum to lastrequest.inc File Changes

M addons/sourcemod/scripting/include/hosties.inc (7) M addons/sourcemod/scripting/include/lastrequest.inc (7) Patch Links:

https://github.com/dataviruset/sm-hosties/pull/63.patch https://github.com/dataviruset/sm-hosties/pull/63.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

Bara commented 7 years ago

Maybe you get a error for this line: https://github.com/dataviruset/sm-hosties/blob/a194ce9ba9f26964eb832461e1d1c82ca0b2e13e/addons/sourcemod/scripting/sm_hosties.sp#L77

data-bomb commented 7 years ago

@DrKittens

https://github.com/dataviruset/sm-hosties/commit/4aae1760ff0fb406b44aaa32c94b56217ae3d9c5 Resolves this issue. The GameType will be recognized before hand by any plugin that includes lastrequest.inc