openmultiplayer / openmultiplayer/open.mp

Some requested changes by @Y-Less

Open
#414 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
641
Forks
208
PR merge metrics
No merged PRs in 30d

Description

@Y-Less proposed some changes in his PR in #377
most of them are accepted, but I reverted a few before merging, so we can discuss about them and not be a reason to hold and keep that PR open.
All these reverted changes are now here to be discussed about, and later on create a PR for it and do required and chosen changes.

TextDrawStyle was changed to TextDrawFont

Currently not called TextDrawFont in master branch, but it's what @Y-Less was offering.
That was done by @Y-Less because he thinks this way people using SDK and known to samp dev environment can understand it better.
Me and @Cheaterman on the other hand had different opinion on this, thinking a change due to old term usage in Pawn scripting API of samp is not really necessary and we can now use new and proper terms in our SDK, keeping what we promised backward compatible along side with it (Pawn scripting API), as a step to future plans and having consistent and proper naming, without hurting our backward compatibility goal at the moment

onPlayerClose(Player)TextDraw was offered

Y_Less also offered to use onPlayerClose(Player)TextDraw as the event name for when player "cancels" textdraw selection, I've renamed it to onPlayerCancel(Player)TextDrawSelection for now, and put this up for discussion so we can decide
In my opinion, "close textdraw" has no meaning, even to those who know what it does it just doesn't sound right, and newcomers won't necessarily understand it at first glance, what does closing a textdraw even mean? even if we talk about in game behavior, it doesn't "close" or hide anything, it simply exists selection state, hence being renamed to "cancel textdraw selection"

Renaming toggleOtherNameTag to useNameTag, but it's confusing and nowhere near being self explanatory

While many others are changed and followed the same style, and are now in master branch, I don't think this one is helping anyone
maybe toggle isn't the best word, but yet useNameTag is very confusing, for being a method of IPlayer
what does player.useNameTag supposed to do? who's name tag is it going to hide? for who? hence keeping it as toggleOtherNameTag for now, so we can decide what to call it

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review PR #377 alongside the current master branch and compare the named TextDraw and name-tag APIs with the reverted changes. Resolve the competing naming proposals and record the chosen API names; done means the decisions are agreed and ready for a follow-up PR.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
api
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.