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 'every' parameter to setElementDimension #111

Merged
merged 1 commit into from
Feb 22, 2017

Conversation

zneext
Copy link
Contributor

@zneext zneext commented Jan 6, 2017

While developing my server (which has multiple lobbies (same gamemode but in different dimensions)) i realized that there wasn't any "easy" way to make an object visible in every dimension (and thus not having to create every object in every dimension or dealing with setElementDimension) so i came up with this, it allows us to use setElementDimension ( object, "every" ) to make an object visible in every dimension. It also reads dimension="every" from .map files.

It's my first time dealing with serverside functions which call their client counterpart so i don't know if i did everything correct, Please point any mistakes i've made. 😃

@ccw808
Copy link
Member

ccw808 commented Jan 8, 2017

Good idea.
Although I prefer "all" instead of "every"

@qaisjp qaisjp added the enhancement New feature or request label Jan 8, 2017
@qaisjp qaisjp force-pushed the setElementDimension-every branch 5 times, most recently from 2a431f1 to 9276b42 Compare January 8, 2017 05:05
@qaisjp
Copy link
Contributor

qaisjp commented Jan 8, 2017

CPacketHandler.cpp and CEntityAddPacket.cpp both had irrelevant changes to whitespace - I have modified your original commit to undo your whitespace changes.

Ignore the "cached" commits shown by GitHub above - they are not actually in the branch... whilst making the modifications I made reverts, but decided to modify your original commit instead (to preserve history). Sorry about the potential email spam.

@qaisjp qaisjp force-pushed the setElementDimension-every branch 2 times, most recently from bf9644c to 942165a Compare January 8, 2017 05:16
@jushar
Copy link
Contributor

jushar commented Jan 8, 2017

I think we should go for getElementVisibleInAllDimensions/setElementVisibleInAllDimensions instead as letting getElementDimension return a string could cause issues if scripts do something like this:

local nextDimension = getElementDimension(element) + 1

Generally speaking, I like the idea though.

@zneext
Copy link
Contributor Author

zneext commented Jan 8, 2017

@ccw808 English isn't my first language, so i didn't knew what word was the correct so i ended up using every, but we can change that if needed.

@qaisjp Thanks for fixing that. VS messed up the withespaces and i didn't saw it, sorry.

@Jusonex When the programmer choose use the parameter every anywhere he should know that getElementDimension will return "every", that's the expected behaviour, I think that introducing a new function for this will be strange, i.e. what is suposed to happen when you make the element visible in every dimension and then try to change its dimension? It would return false? and what about getElementDimension? it would return the previously set dimension when in reality the element isn't in a specific dimension?

Thanks for the feedback guys 😃

@Bonsai11
Copy link

Bonsai11 commented Jan 8, 2017

I'm just curious, why do you even need this for your multigamemode system?

Usually each arena has its own dimension so you won't see other players.
Objects are probably created clientside anyway so it doesn't matter if they are visible in all dimensions or not, as the localPlayer can only be in one anyway

.

@qaisjp
Copy link
Contributor

qaisjp commented Jan 8, 2017

@zneext a possible answer to your question


obj:setDimension(dim) will disable the "dimensionless" state and make it only visible in dim. If an object is "dimensionless", then doing obj:getDimension() will return the original dimension it was in.

i.e, setting the dimension to 3, then making it visible in all dimensions will,

  • when getting the dimension: return 3
  • when setting the dimension: make it no longer visible in all dimensions, and update the dimension to the given dimension

@darkdreamingdan
Copy link
Member

Why don't we assign -1 to 'dimensionless'?

@zneext
Copy link
Contributor Author

zneext commented Jan 9, 2017

@Bonsai11 What do you mean? It's not a multigamemode system, it has only one gamemode but different lobbies, with different matches running on, That's why i came up with this, otherwise i'd have to either create the object on every dimension i need or set all the objects' dimension right after the player join a lobby, Sorry if i haven't made it clear enought on the first post :)

I think that using -1 instead of "every" is the best solution, we won't need to create another functions and it will be compatible with older codes (as getElementDimension would return a number too), WDYGT?

@zneext
Copy link
Contributor Author

zneext commented Feb 9, 2017

Please let me know how you guys want this, I'd love to get this feature in the next update if possible.

@ccw808
Copy link
Member

ccw808 commented Feb 9, 2017

What about a range instead:

setElementDimension( object, first, last )
first, last = getElementDimension( object )

@Bonsai11
Copy link

Bonsai11 commented Feb 9, 2017

Doesn't it cause problems if an element can be in multiple dimensions?

If a player could be two dimensions, he would be visible to players of each of these dimensions.
So if a player in dimension A kills him, player in dimension B sees him being killed out of nowhere?

In case its only about objects, I think there might be a similar problem with objects that can be destroyed or explode. A vehicle crashes into a multi dimensional barrel and it will explode for everyone, even though only the driver's dimension can see him crash.

@karimcambridge
Copy link

"What about a range instead:"

Yea but then what if you you want it visible in dimension 1,3, 5?

I think you should go for an array, which is customisable.

If the array contents equals to -1, then every dimension.

@ccw808
Copy link
Member

ccw808 commented Feb 9, 2017

The advantage of a range over an array is we can avoid -1 (which seems a bit hacky). However using range/array is overcomplicating things, so let's just go with -1

@0x452
Copy link

0x452 commented Feb 10, 2017

Good job!

@zneext
Copy link
Contributor Author

zneext commented Feb 13, 2017

Sorry for the delay, I've had some problems with this branch so today i rewrote this entire feature, New PR: #112

@zneext zneext closed this Feb 13, 2017
@qaisjp
Copy link
Contributor

qaisjp commented Feb 14, 2017

In the future, instead of fragmenting conversation and making multiple pull requests, ask for assistance.

Run these commands on the command line, it should fix the issue:

git checkout master
git branch -D setElementDimension-every
git checkout setElementDimension-all
git branch -m setElementDimension-every
git push origin setElementDimension-every --force

If your remote is not called origin, you may want to change it to whatever points to your own fork.

@qaisjp qaisjp reopened this Feb 14, 2017
@zneext
Copy link
Contributor Author

zneext commented Feb 14, 2017

Sorry about that @qaisjp, Running these commands fixed the issue.

@qaisjp
Copy link
Contributor

qaisjp commented Feb 14, 2017

That's great to hear! No problem.

@ccw808 ccw808 self-requested a review February 21, 2017 20:49
@ccw808 ccw808 merged commit 2e319aa into multitheftauto:master Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants