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

Add GameData methodmap #766

Merged
merged 1 commit into from Oct 12, 2018
Merged

Conversation

@peace-maker
Copy link
Member

@peace-maker peace-maker commented Feb 5, 2018

Wrap gamedata natives in a pretty methodmap and update stock plugins/includes to use it.

This fixes bad documentation on GameConfGetAddress and not closing the gameconfig file if handle creation fails as well.

@asherkin
Copy link
Member

@asherkin asherkin commented Feb 6, 2018

I'm not a fan of the name GameConf, but don't really have a better suggestion, opening up to the floor? @alliedmodders/sourcemod

Implementation looks sounds, nice catch on the fixes.

@peace-maker
Copy link
Member Author

@peace-maker peace-maker commented Feb 6, 2018

Could call it GameData since that's what people use to refer to it.

@Fyren
Copy link
Contributor

@Fyren Fyren commented Feb 6, 2018

I think with the existing natives named the way they are, I'd prefer GameConf.

@Headline
Copy link
Member

@Headline Headline commented Feb 7, 2018

Personally, I think that naming it GameData is a better choice even though it differs from the original native's naming scheme. It looks cleaner, and this is an evolving language anyway. 😛

I just wanted to give my two cents, if that's welcome.

Wrap gamedata natives in a pretty methodmap.

This fixes bad documentation on `GameConfGetAddress` and not closing the gameconfig file if handle creation fails as well.
@peace-maker peace-maker force-pushed the gameconf_methodmap branch from ab99b8a to 5728795 Apr 15, 2018
@peace-maker peace-maker changed the title Add GameConf methodmap Add GameData methodmap May 26, 2018
@psychonic
Copy link
Member

@psychonic psychonic commented Jul 8, 2018

I have no strong feelings one way or the other on the naming. Everything else looks okay to me.

Copy link
Member

@Headline Headline left a comment

this lgtm; much cleaner and is a good addition to the mm api. 🥂

@Headline Headline merged commit 1b795a7 into alliedmodders:master Oct 12, 2018
2 checks passed
@peace-maker peace-maker deleted the gameconf_methodmap branch Oct 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants