mtasa-blue icon indicating copy to clipboard operation
mtasa-blue copied to clipboard

Add new world special property (burn flipped cars)

Open samr46 opened this issue 3 years ago • 9 comments

Solves #1076

Adds new property "burnflippedcars" to functions: setWorldSpecialPropertyEnabled, isWorldSpecialPropertyEnabled. Default: true (cars start to burn when upside down; current behavior)

If set to false then cars will not start to burn when upside down.

setWorldSpecialPropertyEnabled("burnflippedcars", false)

It doesn't affect any other vehicle health related behavior. The vehicles can be damaged and health can be changed using setElementHealth. Will be useful to make specific vehicles burn upside down if needed.

Test resource: burnflippedcars.zip (there is README.txt inside with some notes and suggestions about testing)

samr46 avatar Sep 22 '21 12:09 samr46

Out of curiousity, how does the sync work? For example client 1 has this world property enabled, client 2 has it disabled. The car starts burning depending on who is the syncer?

ffsPLASMA avatar Sep 22 '21 14:09 ffsPLASMA

Out of curiousity, how does the sync work? For example client 1 has this world property enabled, client 2 has it disabled. The car starts burning depending on who is the syncer?

good question from experience burning cars without a syncer don't explode at all

LosFaul avatar Sep 22 '21 14:09 LosFaul

For example client 1 has this world property enabled, client 2 has it disabled. The car starts burning depending on who is the syncer?

Yes, it would start burning for the player that has this feature set to true (and if this player is current syncer then it will start burning for everyone). Only the syncer notifies the server about vehicle health change so it wouldn't look good for players with different options. It's a client side feature (like other world special properties). And several existing world properties can cause visual sync related issues as well if not used for all players.

Normally it should be used for example in onClientResourceStart event. At least, this is intended usage and i don't see any reason someone would like to do it differently.

I also don't see any need to make it sever side with extensive sync handling and essentially vehicle specific. Making it work this way would require hooking several functions with additional logic (completely unnecessary complications).

samr46 avatar Sep 22 '21 14:09 samr46

I also don't see any need to make it sever side with extensive sync handling and essentially vehicle specific. Making it work this way would require hooking several functions with additional logic (completely unnecessary complications).

Agreed.

Would it be difficult to make unoccupied and ped occupied vehicles ignite when flipped?

Zangomangu avatar Sep 22 '21 20:09 Zangomangu

Would it be difficult to make unoccupied and ped occupied vehicles ignite when flipped?

You mean by patching the game itself or using Lua? Because it could be done using Lua easily and more. This is what i had in mind for this feature:

  • You prevent all vehicles from burning while flipped using this new property;
  • Then if you need specific vehicles to burn while flipped you simply use setElementHealth to set it on fire. Basic timer that checks streamed-in synced by local player vehicles and applies new health gradually till 0 if vehicle rotation suggests that it is upside down.

I use similar approach right now to prevent vehicles from burning flipped (rotation check + setVehicleDamageProof). The downside of this approach is that it makes the vehicle invincible (it could be solved using custom damage system tho). This new property essentially replacing this whole system without any additional changes in my case.

The game could be patched to handle occupied and unoccupied flipped vehicles differently. But i think there is no need to do it since we will have full control over it with Lua anyway.

samr46 avatar Sep 22 '21 21:09 samr46

Yeah I meant patching the game, but your approach makes sense, so I agree there's no need

Zangomangu avatar Sep 22 '21 22:09 Zangomangu

Cool feature. Works for me.

TheNormalnij avatar Jan 03 '22 16:01 TheNormalnij

Yeah, it would be nice to merge it. As soon as someone reviews it (should be easy).

samr46 avatar Jan 03 '22 19:01 samr46

code looks good for me. I like this feature, especialy because it can help optimize common thing people are doing

CrosRoad95 avatar Jun 05 '22 17:06 CrosRoad95

We tested this feature, and this works fine for one player, but in situation with several players, it need to be turned to all i suppose.

I can say that it is strange behavior, but setWorldSpecialPropertyEnabled works same with another parameters like snipermoon, so it's OK.

Disinterpreter avatar Nov 19 '22 16:11 Disinterpreter

committee made out of 5 people tested in detail following feature and expressed a positive opinion on its operation image

CrosRoad95 avatar Nov 19 '22 16:11 CrosRoad95

This feature works only when all players on server have it enabled.

Also there is a race condition. When player download server resources, he can burn vehicles in sync distance.

I think, it should be serverside function. You should send flag bWorldBurnFlippedVehicles in join packet and a packet when you enable this feature. In this case it'll works perfect and i can merge this PR.

TheNormalnij avatar Nov 19 '22 17:11 TheNormalnij

This feature works only when all players on server have it enabled.

This is exactly how I saw it used. It was discussed earlier in the conversation.

Also there is a race condition. When player download server resources, he can burn vehicles in sync distance.

This edge case can be easily avoided using download_priority_group in meta.xml (like using some dummy resource without files). But it will not be needed for most servers anyway because normally you need to set this once before player spawns.

I think, it should be serverside function. You should send flag bWorldBurnFlippedVehicles in join packet and a packet when you enable this feature. In this case it'll works perfect and i can merge this PR.

I personally think that it's completely unnecessary. But if this is the only way for it to be merged then maybe someone will be interested in implementing server side function. I'm not gonna be able to do this anytime soon.

samr46 avatar Nov 19 '22 17:11 samr46

This edge case can be easily avoided using download_priority_group in meta.xml (like using some dummy resource without files). But it will not be needed for most servers anyway because normally you need to set this once before player spawns.

Yeah, i know about it, but default camera position can still trigger this issue. Serverside function will be simple API for server developers w/o additional steps, that developer can miss

TheNormalnij avatar Nov 19 '22 18:11 TheNormalnij

Serverside function will be simple API for server developers w/o additional steps, that developer can miss

I guess we could use setGlitchEnabled for this instead of adding new functions. Something like "dontburnflippedcars". Not sure about the name.

samr46 avatar Jan 23 '23 18:01 samr46

I think the point about race condition trigger is valid and hence would be necessary to implement a server synchronized setting for this before merging.

patrikjuvonen avatar Apr 08 '23 13:04 patrikjuvonen

Function changed to setGlitchEnabled ("dontburnflippedcars") Usage: setGlitchEnabled("dontburnflippedcars", true)

samr46 avatar Apr 08 '23 22:04 samr46

@samr46 Please update the wiki page for glitches.

botder avatar Jul 25 '23 15:07 botder

I like the idea of making this feature toggleable through server functions, but shoehorning it into setGlitchEnabled feels a bit odd to me, since it's not a singleplayer glitch fix. It's hard to discover that setGlitchEnabled is capable of toggling this by skimming through the wiki function lists.

How feasible would it be to move it to setWorldSpecialPropertyEnabled and make that function shared? The latter is a useful quality of life improvement, as several typical uses of setWorldSpecialPropertyEnabled change properties for all clients anyway.

AlexTMjugador avatar Jul 25 '23 15:07 AlexTMjugador

@AlexTMjugador That sounds right. It's time to add setWorldSpecialPropertyEnabled to the server and move 'dontburnflippedcars' there.

botder avatar Jul 25 '23 16:07 botder

Alright! @samr46, would you like to file a separate PR for that? I wish I had reviewed this before the merge, but I didn't notice the change to use setGlitchEnabled until now, sorry :sweat_smile:

AlexTMjugador avatar Jul 25 '23 16:07 AlexTMjugador

Please don't bother, I am already working on that.

botder avatar Jul 25 '23 16:07 botder

Alright! @samr46, would you like to file a separate PR for that? I wish I had reviewed this before the merge, but I didn't notice the change to use setGlitchEnabled until now, sorry 😅

Just created new branch 😂 But it will be faster for @botder to do it and merge right away. I will edit wiki again to reflect this afterwards.

samr46 avatar Jul 25 '23 16:07 samr46

@samr46 Sorry for this, but can you do it instead?

botder avatar Aug 14 '23 22:08 botder

Sorry for this, but can you do it instead?

Yes, I will send new PR ~ tomorrow.

samr46 avatar Aug 16 '23 17:08 samr46