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

Editor ribbon cannot be expanded once collapsed #1292

Closed
crabmusket opened this Issue Apr 29, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@crabmusket
Contributor

crabmusket commented Apr 29, 2015

Clicking the little arrow on the ribbon to collapse the icons works, but clicking it again to expand them fails with a console message. EDIT: introduced in #1092, I think. EDIT: reverting 61e1062 made no difference. Hmm.

@crabmusket crabmusket added the Bug label Apr 29, 2015

@crabmusket crabmusket added this to the 3.7 milestone Apr 29, 2015

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket May 9, 2015

Contributor

There's code that looks like this:

%this.setExtent((29 + 4) * %count + 12, 33);

Which is somehow causing setExtent to set the extent to 33, 33. Um, what? It works if I do this:

%x = (29 + 4) * %count + 12;
%this.setExtent(%x, 33);

And also if I change setExtent to take Strings instead of const char*s. @jamesu could this be one of yours?

Contributor

crabmusket commented May 9, 2015

There's code that looks like this:

%this.setExtent((29 + 4) * %count + 12, 33);

Which is somehow causing setExtent to set the extent to 33, 33. Um, what? It works if I do this:

%x = (29 + 4) * %count + 12;
%this.setExtent(%x, 33);

And also if I change setExtent to take Strings instead of const char*s. @jamesu could this be one of yours?

crabmusket added a commit to crabmusket/Torque3D that referenced this issue May 9, 2015

setExtent now takes Strings instead of leaving args as pointers.
It appears that passing raw expressions as arguments (e.g.
setExtent(..exp.., 4)) was causing the extOrX value to be the same as y,
which was unhelpful and bizarre. This fixes it, but I'm not sure how.
See GarageGames#1292.
@jamesu

This comment has been minimized.

Show comment
Hide comment
@jamesu

jamesu May 13, 2015

Contributor

I'll have a look and see whats going on

Contributor

jamesu commented May 13, 2015

I'll have a look and see whats going on

@jamesu

This comment has been minimized.

Show comment
Hide comment
@jamesu

jamesu May 15, 2015

Contributor

Ok the problem is with these fancy DefineConsole* methods, when it unmarshalls and converts those two values to strings according to the function definition it uses the same buffer (i.e. Con::getReturnBuffer) to store the value, thus each value becomes 33.

Accepting String instead is a reasonable workaround, though really the console code should print these values to a temporary buffer space since quite a few functions seem to use const char* as parameters.

I'll see if I can cook up a good workaround.

Contributor

jamesu commented May 15, 2015

Ok the problem is with these fancy DefineConsole* methods, when it unmarshalls and converts those two values to strings according to the function definition it uses the same buffer (i.e. Con::getReturnBuffer) to store the value, thus each value becomes 33.

Accepting String instead is a reasonable workaround, though really the console code should print these values to a temporary buffer space since quite a few functions seem to use const char* as parameters.

I'll see if I can cook up a good workaround.

@jamesu

This comment has been minimized.

Show comment
Hide comment
@jamesu

jamesu May 15, 2015

Contributor

Ok I have a fix, just testing and I'll submit a PR.

Contributor

jamesu commented May 15, 2015

Ok I have a fix, just testing and I'll submit a PR.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket May 15, 2015

Contributor

Awesome, really appreciate it! I submitted the String fix in case you didn't have time to look into it, but we have a couple of other things to do for 3.7 so hopefully we can get the proper fix in. I'm surprised this hasn't turned up as a problem elsewhere.

Contributor

crabmusket commented May 15, 2015

Awesome, really appreciate it! I submitted the String fix in case you didn't have time to look into it, but we have a couple of other things to do for 3.7 so hopefully we can get the proper fix in. I'm surprised this hasn't turned up as a problem elsewhere.

jamesu added a commit to jamesu/Torque3D that referenced this issue May 15, 2015

@jamesu

This comment has been minimized.

Show comment
Hide comment
@jamesu

jamesu May 15, 2015

Contributor

Pushed fix. Ideally this problem should be resolved in a better way but this will do for now.

Contributor

jamesu commented May 15, 2015

Pushed fix. Ideally this problem should be resolved in a better way but this will do for now.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket May 15, 2015

Contributor

Thanks for that. I'm going to test this and revise the other outstanding PRs tomorrow.

Contributor

crabmusket commented May 15, 2015

Thanks for that. I'm going to test this and revise the other outstanding PRs tomorrow.

Areloch added a commit that referenced this issue May 25, 2015

crabmusket added a commit that referenced this issue May 28, 2015

@crabmusket crabmusket closed this May 28, 2015

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