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: Expand place options #448

Merged
merged 9 commits into from
Jan 6, 2018
Merged

Add: Expand place options #448

merged 9 commits into from
Jan 6, 2018

Conversation

1kuemmel1
Copy link

@1kuemmel1 1kuemmel1 commented Dec 15, 2017

  • Add: Expand place options.

When merged this pull request will:

  • Add a new placing option: tilt
  • Changed the handling of rotating and tilting of an object
  • Use ACE3 Mousehint for release action
  • Remove the old addAction entry
  • Adaptation of stringtable.xml
  • Changed handling of object status in database (save/load)
    • use getPosWorld and setPosWorld instead of getPosASL and setPosASL
    • added some comments
    • handle setVectorDirAndUp on database save and load

Final test:

  • local
  • server
  • database handling of objects (save/load)

Screenshots
placeing_new_full
placing_new_releaseplacing_new_hint

some changings to place.sqf and place_key_down.sqf
@Vdauphin Vdauphin added this to the 1.18 milestone Dec 15, 2017
@Vdauphin
Copy link
Owner

Hello, very good idea but you also need to make it persistent (/core/fnc/db/save.sqf#L133 and /core/fnc/db/load.sqf#L287).

I notice you use a lot of brackets. I am following this coding guidelines for those : https://ace3mod.com/wiki/development/coding-guidelines.html#55-brackets-around-code. Can you also follow it ?
For exemple : [(localize "STR_BTC_HAM_LOG_PLACE_RELEASE"),""] call ace_interaction_fnc_showMouseHint; //Release
Juste write : [localize "STR_BTC_HAM_LOG_PLACE_RELEASE", ""] call ace_interaction_fnc_showMouseHint; //Release

Thanks see you soon !

@1kuemmel1
Copy link
Author

Hi, I've updated some thinks.

I notice you use a lot of brackets. I am following this coding guidelines for those : https://ace3mod.com/wiki/development/coding-guidelines.html#55-brackets-around-code. Can you also follow it ?

Sorry for this.
Did I make it correct now?

I've modified the loadObjectStatus.sqf and saveObjectStatus.sqf to handle vector settings.

Database saved:
beforedb_save

Database loaded:
afterdb_save

Update first comment in PR.

@Vdauphin
Copy link
Owner

This looks perfect ! I will review it soon. Just some questions :

@Vdauphin Vdauphin self-requested a review December 16, 2017 12:28
@1kuemmel1
Copy link
Author

Why do you use getPosWorld and setPosWorld instead of getPosASL and setPosASL ?

Because PositionWorld referenced to model center. This is better (more accurate) for setVectorDirAndUp. And BIS said: But new to Arma 3 command getPosWorld is the fastest of them all. 😄 (see: Code Optimisation see: Position World is the fastest)

Does the setVectorDirAndUp is correctly broadcasted between client on dedicate server? I noticed for C4 on suicider there was not even if the Wiki say yes :/ (#282).

On a locally hosted dedicated server, everything seems to be fine. I'm going to do a little test on a dedicated server soon. (Tomorrow or so).

@1kuemmel1
Copy link
Author

Update:
After a little test on a dedicated server, I can say: it's going quite well so far.

Saved to database:
savedb_dedi

After server restart (loaded from database):
loaddb_dedi

@1kuemmel1 1kuemmel1 closed this Dec 17, 2017
@1kuemmel1
Copy link
Author

Ahh sorry, I hit the wrong button.😕 😃

@1kuemmel1 1kuemmel1 reopened this Dec 17, 2017
@Vdauphin
Copy link
Owner

Good job ! looks like it works prefectly!

I forgot to ask you to increment the mission version to prevent data base conflict with previous version (1.17). To do so just write 1.171 in https://github.com/Vdauphin/HeartsAndMinds/blob/master_daily/%3DBTC%3Dco%4030_Hearts_and_Minds.Altis/core/def/mission.sqf#L4

@1kuemmel1
Copy link
Author

I forgot to ask you to increment the mission version to prevent data base conflict with previous version (1.17).

done

Copy link
Owner

@Vdauphin Vdauphin left a comment

Choose a reason for hiding this comment

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

Looks very good! I will do final test soon.

…o ADD-overhaul_place_options

# Conflicts:
#	=BTC=co@30_Hearts_and_Minds.Altis/core/def/mission.sqf
@Vdauphin Vdauphin merged commit 532ab64 into Vdauphin:master_daily Jan 6, 2018
detach btc_log_placing_obj;

//save to DB
[btc_log_placing_obj] call btc_fnc_db_saveObjectStatus;
Copy link
Owner

Choose a reason for hiding this comment

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

@1kuemmel1 Why do you use btc_fnc_db_saveObjectStatus here ? You are doing nothing, right ?

Copy link
Author

Choose a reason for hiding this comment

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

Why?... actually no idea. But you be right it's useless there. Just remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants