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 interface for model flags #1714

Merged
merged 42 commits into from Apr 7, 2023

Conversation

TheNormalnij
Copy link
Contributor

@TheNormalnij TheNormalnij commented Oct 14, 2020

API:

bool engineSetModelFlags( int modelID, int flags [, bool bIdeFlags] )
int engineGetModelFlags( int modelID )
bool engineResetModelFlags( int modelID )
bool engineGetModelFlag( int modelID, string flagName )
bool engineSetModelFlag( int modelID, string flagName, bool state )

Flags:

is_road
draw_last
addictive
ignore_lighting
no_zbuffer_write
dont_receive_shadows
is_glass_type_1
is_glass_type_2
is_garage_door
is_damagable
is_tree
is_palm
does_not_collide_with_flyer
is_tag
disable_backface_culling
is_breakable_statue
is_crane

Flags documentation

Important
engineGetModelFlags returns internal GTA flags.
You can transform IDE flags into GTA flags with 3 argument in engineSetModelFlags.

@StrixG StrixG added the enhancement New feature or request label Oct 14, 2020
@StrixG StrixG added this to the 1.6 milestone Oct 14, 2020
Client/game_sa/CModelInfoSA.h Outdated Show resolved Hide resolved
Client/game_sa/CModelInfoSA.cpp Outdated Show resolved Hide resolved
TheNormalnij and others added 2 commits October 26, 2020 20:50
Co-authored-by: Nikita Obrekht <obrekht@gmail.com>
Co-authored-by: Nikita Obrekht <obrekht@gmail.com>
@StrixG StrixG modified the milestones: 1.6, Confirmed Issues Oct 26, 2020
@TheNormalnij
Copy link
Contributor Author

Can we merge this?

@Woovie
Copy link
Contributor

Woovie commented Nov 2, 2020

I don't feel it's ready for merge. I tested this codebase while @qaisjp watched as well. The API being provided for get/set is not adequate. It's difficult to use, confusing, and generally not user friendly. We don't have a recommendation/solution for this, but something should change to make it easier for people to use.

@dmi7ry
Copy link

dmi7ry commented Nov 2, 2020

I don't feel it's ready for merge. I tested this codebase while @qaisjp watched as well. The API being provided for get/set is not adequate. It's difficult to use, confusing, and generally not user friendly. We don't have a recommendation/solution for this, but something should change to make it easier for people to use.

Do you mean bitwise operations? If so then it would be possible to add functions engineSetModelFlag(modelID, "flag", state) and engineGetModelFlag(modelID, "flag"). Like engineSetModelFlag(modelID, "isRoad", true).

@TheNormalnij
Copy link
Contributor Author

Table with all model flags

@Woovie
Copy link
Contributor

Woovie commented Nov 3, 2020

Table with all model flags

Private sheet

@TheNormalnij
Copy link
Contributor Author

Fixed

@lopezloo
Copy link
Member

lopezloo commented Oct 14, 2021

Do all these flags actually work? is_crane for example. What does it do?

@TheNormalnij
Copy link
Contributor Author

I saw almost all flags in IDA, but im not sure that all of them have effect in MTA. My "must have" list:

draw_last -- fix world models culling
addictive -- fix render issues
ignore_lighting
no_zbuffer_write
dont_receive_shadows
is_palm

@Shuubaru
Copy link

i'm curious, will this fix vehicle/peds shadows over non-world objects created with createObject function?

@Allerek
Copy link
Contributor

Allerek commented Oct 23, 2021

Is there any docs about what these flags do? We can guess some for sure like
is_palm is probably about bending the object while wind is strong etc.

@lopezloo
Copy link
Member

lopezloo commented Oct 23, 2021

i'm curious, will this fix vehicle/peds shadows over non-world objects created with createObject function?

It won't. MTA created objects are treated by game as small dynamic objects like lampposts etc. and that's why they do not reflect shadows.

@Lpsd
Copy link
Member

Lpsd commented Jan 8, 2022

As briefly mentioned on Discord - I think the API needs simplifying and the usage being more "consistent".

We should have only 2 functions for setting and getting flag(s). Each function you can either pass a single flag ID, or a table of flag IDs (ints).

Also, both functions should accept/return the flag IDs (ints). This is the consistency part, as currently you have to work with both ints as well as string names, depending on if you're setting one, or multiple flags.

We can have a table of the flag IDs and their names on the wiki.

This also gets rid of the need for that bIdeFlags arg.

Ideally, the functions would be:

table engineGetModelFlags( int modelID, int flagID / table flagIDs )
bool engineSetModelFlags( int modelID, int flagID / table flagIDs, bool state )

@TheNormalnij
Copy link
Contributor Author

As i remember, GTA SA translates flags from IDE files in internal format, and we can't translate GTA format in IDE format.
I think map developers know better ide flags and will be discouraged with internal gta system.
Also people will use precalculated flags from ide files or MTA editors, and this is main usage, i think.

So, user friendly API should handle well-known information in this situation.

As Pirulax said once "No magic numbers in API". We have some unfriendly examples like get/setVehicleWheelsSates. I always check wiki about these functions. In my opinion flagID will be fast, but harder to use.

Current API is perfect for 2 types of operations:

  1. You need set flags for a new object. You use engineSetModelFlags with your precalculated value or you get this value from bitOr with enabled flags from IDE or GTA.
  2. You need tweak objects. If you need change only one flag, or disable/enable some flags for all models, engineSetModelFlag will help you.

@Zangomangu
Copy link
Contributor

As i remember, GTA SA translates flags from IDE files in internal format, and we can't translate GTA format in IDE format.

If we can't translate it back, I don't think we should expose it. Most people would set flags in the IDE format, because that's the documented format.
So you set a value, but you get a different value. That causes confusion. Is there any way we can just return the IDE formatted flags? GTA shouldn't be changing the model flags, so only this function would be altering them?

As pointed out above, current scripting functions take alternative parameters instead of having several functions. So we should keep it to one set/get/reset IMO.

I also agree we shouldn't come up with magic numbers, so what about the following.
Scripters not familiar with bitwise operations can use the string variant to set/get individual flags.

int flags for the IDE flags.
string flagName for a specific flag.

int/bool engineGetModelFlags( int modelID [, string flagName ] )

Returns IDE flags, or state of a specific flag if flagName is specified.
Function should error if parsed an invalid modelID to avoid ambiguity.

bool engineSetModelFlags( int modelID, int flags / string flagName [, bool state ] )

Sets IDE flags, or a specific flag if flagName and state is specified

@gta191977649
Copy link
Contributor

As i remember, GTA SA translates flags from IDE files in internal format, and we can't translate GTA format in IDE format.

If we can't translate it back, I don't think we should expose it. Most people would set flags in the IDE format, because that's the documented format. So you set a value, but you get a different value. That causes confusion. Is there any way we can just return the IDE formatted flags? GTA shouldn't be changing the model flags, so only this function would be altering them?

As pointed out above, current scripting functions take alternative parameters instead of having several functions. So we should keep it to one set/get/reset IMO.

I also agree we shouldn't come up with magic numbers, so what about the following. Scripters not familiar with bitwise operations can use the string variant to set/get individual flags.

int flags for the IDE flags. string flagName for a specific flag.

int/bool engineGetModelFlags( int modelID [, string flagName ] )

Returns IDE flags, or state of a specific flag if flagName is specified. Function should error if parsed an invalid modelID to avoid ambiguity.

bool engineSetModelFlags( int modelID, int flags / string flagName [, bool state ] )

Sets IDE flags, or a specific flag if flagName and state is specified

I agreed, most people will use the flags id from corresponding idea file, this feature will benefit especially someone who are trying to do a map total convertion in MTA.

@patrikjuvonen

This comment was marked as resolved.

@patrikjuvonen patrikjuvonen added the feedback Further information is requested label Dec 24, 2022
@patrikjuvonen patrikjuvonen removed the feedback Further information is requested label Jan 1, 2023
patrikjuvonen
patrikjuvonen previously approved these changes Jan 1, 2023
Copy link
Contributor

@patrikjuvonen patrikjuvonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you very much.

patrikjuvonen
patrikjuvonen previously approved these changes Apr 7, 2023
@patrikjuvonen patrikjuvonen merged commit ec314df into multitheftauto:master Apr 7, 2023
6 checks passed
@TheNormalnij TheNormalnij deleted the model_flags branch April 9, 2023 15:59
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.

None yet