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.

Sign up for GitHub

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

Jump to bottom

Raise lower carry object #5234

Merged
merged 8 commits into from
Jun 4, 2017
Merged

Raise lower carry object #5234

merged 8 commits into from
Jun 4, 2017

Conversation

Phyma
Copy link
Contributor

@Phyma Phyma commented Jun 2, 2017

When merged this pull request will:

  • Will fix bug where you cant raise or lower some objects
  • Doesn't use setPosATL but uses the internal ace_dragging_carryingPos

@Phyma Phyma mentioned this pull request Jun 2, 2017
Closed
@PabstMirror
Copy link
Contributor

This has some problems with being able to scroll the item below ground and breaks on objects manually set with ace_dragging_fnc_setCarryable.

But it shows that the real problem is setPosATL doesn't have immediate effects when object is not local.
So I think a better solution would be to just replace handleScrolWheel 40,41 with

private _positionChange =  _position vectorDiff (getPosATL _carriedItem);
private _selectionPosition = _unit worldToModel (ASLtoAGL getPosWorld _carriedItem);
_selectionPosition = _selectionPosition vectorAdd _positionChange;
_carriedItem attachTo [_unit, _selectionPosition];

@Phyma
Copy link
Contributor Author

Phyma commented Jun 2, 2017 •
edited
Loading

Is that code tested on dedicated? Dosent work at all. The item just disappears.
Can check later what is wrong with that code.

And putting it to low is something i also noticed but it will be put on the ground not in it. Dont see what breaks in setCarryable that I changed :/ but also i'm learning so

PabstMirror
PabstMirror reviewed Jun 2, 2017
View reviewed changes
addons/dragging/functions/fnc_dropObject_carry.sqf Outdated
// Set to default value
private _parentConfig = [_target] call CBA_fnc_getObjectConfig;
private _defaultPos = getArray(_parentConfig >> QGVAR(carryPosition));
_target setVariable [QGVAR(carryPosition), _defaultPos, true];
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks ace_dragging_fnc_setCarryable because the object has no config, so it stores []

@PabstMirror
Copy link
Contributor

df147b8
Seems to work in MP

@Phyma
Copy link
Contributor Author

Phyma commented Jun 3, 2017

Sorry. I didnt clean all the lines of code. So forgot to change back to the original _position declaration.
Works on Dedicated aswell.

@Phyma
Copy link
Contributor Author

Phyma commented Jun 3, 2017

Still real new to the 100 different ways of taking out a position and what space it is in when you take it out.

@kymckay
Copy link
Member

kymckay commented Jun 3, 2017

I know your pain 😄

@jonpas jonpas added the kind/bug-fix Release Notes: **FIXED:** label Jun 3, 2017
@jonpas jonpas added this to the 3.10.0 milestone Jun 3, 2017
@jonpas
Copy link
Member

jonpas commented Jun 3, 2017

One day you'll think you've grasped it all, just to realize you aren't even close.

jonpas
jonpas previously requested changes Jun 3, 2017
View reviewed changes
Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

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

Very nice solution. I'd also like a comment above this block why setPosATL or something else is not used, as to someone reading this code it might be very confusing why take this approach.

addons/dragging/functions/fnc_handleScrollWheel.sqf Outdated
@@ -37,8 +37,10 @@ _position set [2, ((_position select 2) + _scrollAmount min (_maxHeight + 1.5))

// move up/down object and reattach at current position
detach _carriedItem;
_carriedItem setPosATL _position;
_carriedItem attachTo [_unit];
private _positionChange = _position vectorDiff (getPosATL _carriedItem);
Copy link
Member

Choose a reason for hiding this comment

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

2 spaces on the right side of =.

jonpas
jonpas reviewed Jun 4, 2017
View reviewed changes
addons/dragging/functions/fnc_handleScrollWheel.sqf Outdated
@@ -37,7 +37,9 @@ _position set [2, ((_position select 2) + _scrollAmount min (_maxHeight + 1.5))

// move up/down object and reattach at current position
detach _carriedItem;
private _positionChange = _position vectorDiff (getPosATL _carriedItem);

// Uses this method of selecting position because setATL dosen't work on dedicated server
Copy link
Member

Choose a reason for hiding this comment

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

doesn't

That's not entirely true though, it only doesn't work correctly when object is not local, if I understood the core issue correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tha is true sorry

Copy link
Contributor

Choose a reason for hiding this comment

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

setPosATL does work on remote objects, but from what I saw it didn't have immediate effects
At least in the same frame:

x setPosATL y;
(getPosATL x) != y

@PabstMirror PabstMirror merged commit cec82be into acemod:master Jun 4, 2017
@Phyma Phyma deleted the raiseLowerCarryObject branch June 5, 2017 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PabstMirror PabstMirror PabstMirror approved these changes

@jonpas jonpas jonpas left review comments

Assignees
No one assigned
Labels
kind/bug-fix Release Notes: **FIXED:**
Projects
None yet
Milestone
3.10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@Phyma @PabstMirror @kymckay @jonpas