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 FILE_EXISTS macro to common macros #899

Merged
merged 10 commits into from
Apr 14, 2018
Merged

Add FILE_EXISTS macro to common macros #899

merged 10 commits into from
Apr 14, 2018

Conversation

klemmchr
Copy link
Contributor

When merged this pull request will:

  • Remove the macro FILE_EXISTS from cba settings and add it to common macros

There are some useful cases where a developer would like to load a file but wants to check first if it exists to prevent error screens popping up.

@jonpas
Copy link
Member

jonpas commented Mar 11, 2018

This will throw an error in RPT, might want to add that.

@commy2
Copy link
Contributor

commy2 commented Mar 11, 2018

On machines without interface, this reports always false. This is because the control will be null.

@commy2
Copy link
Contributor

commy2 commented Mar 11, 2018

It will also report false outside of 3den, because it currently adds the dummy control to Display3DEN. It should "work" with RscDisplayMain/idd 0 though.

};

if (_fileExists) then {
if (FILE_EXISTS(_file)) then {
Copy link
Contributor

@commy2 commy2 Mar 11, 2018

Choose a reason for hiding this comment

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

initDisplayMain.sqf is executed in the onLoad event of RscDisplayMain/idd=0. findDisplay 0 will report null, because findDisplay is bugged and only reports displays that were created at least one frame* earlier.
This will cause the macro script to fall back to loadFile. And if loadFile fails, it will create an error message.
This leads to this branch of CBA to create an error pop up when loaded without cba_settings.sqf file in the main directory and file patching enabled. Try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this stay how it was or should there be a macro like FILE_EXISTS_CUSTOM also accepting a display as param to handle such cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could read the main display from uiNamespace:

uiNamespace getVariable "RscDisplayMain"

That should work even during this onLoad event as XEH DisplayLoaded is executed after the BIS_fnc_initDisplay script that sets these variables.
You should also drop the 313 display completely, as the main menu display does exist in 3den.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the main menu also exist when a mission is loaded? If not, does display 313 exist there? How would this behave when neither in 3DEN nor in main menu?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure the main menu display survives the whole session, but not 100% as I haven't messed around with it a lot in (dedicated client) MP.
Display3den is created once you enter the editor and it survives even through previews until you leave the editor back to the main menu.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. But just calling disableSerialization would have the side effect of disabling serilization for the script. Which cannot be intended.
Should wrap the thing into an isNil block to force unscheduled environment I'd say. Or alternatively store the control inside an array or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't disableSerialization scope based? However, in my tests I got no error without disableSerialization which is strange but the macro still seems to work fine. And I think you also didn't use disableSerialization in the settings scoped macro.

Copy link
Contributor

Choose a reason for hiding this comment

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

@chris579 Script based. VM based.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't disableSerialization scope based?

No.

in my tests I got no error

The error requires serialization to be enabled in the first place. Local variables in unscheduled environment are not affected.

Copy link
Contributor

Choose a reason for hiding this comment

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

And I think you also didn't use disableSerialization in the settings scoped macro.

Yes, because it was for this specific component, and I don't ever use the scheduled environment.

@commy2 commy2 added the Feature label Mar 19, 2018
@commy2 commy2 added this to the 3.7 milestone Mar 19, 2018
@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

Needs testing.

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

Any reason to not make this a function instead? The macro uses call anyway.

@commy2 commy2 self-assigned this Mar 19, 2018
@klemmchr
Copy link
Contributor Author

Not really, could really be a function.

@commy2
Copy link
Contributor

commy2 commented Mar 19, 2018

Will do later.

@Killswitch00 Killswitch00 merged commit f1bd2cd into CBATeam:master Apr 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants