trevorsandy / lpub3d

An LDraw™ editor for LEGO® style digital building instructions.
https://trevorsandy.github.io/lpub3d/
133 stars 19 forks source link

Use of NOSTEP seems to cancel BUFEXCHG #533

Closed cjhendrix closed 3 years ago

cjhendrix commented 3 years ago

Subject

When using 0 !LPUB NOSTEP after a BUFEXCHG STORE (just before the BUFEXCHG RETRIEVE), the retrieved version seems to be overridden.

Final step without NOSTEP (assembly is correct): image

Final step with NOSTEP (assembly is incorrect): image

Also worth noting is that when the steps are combined into a multistep with the NOSTEP command, the result doesn't have the problem: image

Environment

Steps to reproduce

This file will have the problem on step 3:

0 Name: main.ldr
0 Author: CJ Hendrix
0 !LPUB FINAL_MODEL_ENABLED GLOBAL FALSE
1 4 -60 40 0 1 0 0 0 1 0 0 0 1 3020.dat
0 STEP
0 BUFEXCHG A STORE
0 !LPUB CALLOUT BEGIN
1 2 -30 -32 0 0 0 -1 0 1 0 1 0 0 3938.dat
1 2 -30 -32 0 0 0 -1 0 1 0 1 0 0 3937.dat
0 !LPUB CALLOUT POINTER LEFT 0.298 1.115 0.159 0.000 0.000 0.000 0.000 0.000 0.000 0.000 1
0 !LPUB CALLOUT PLACEMENT RIGHT CENTER ASSEM OUTSIDE OFFSET 0.5695 -0.0388
0 !LPUB CALLOUT END
1 0 -30 40 0 1 0 0 0 1 0 0 0 1 HasvL2.dat
0 STEP
0 !LPUB NOSTEP
0 BUFEXCHG A RETRIEVE
1 2 -30 16 0 0 0 -1 0 1 0 1 0 0 3938.dat
1 2 -30 16 0 0 0 -1 0 1 0 1 0 0 3937.dat
0 STEP
1 1 -60 8 0 1 0 0 0 1 0 0 0 1 3020.dat
0 STEP
0 !LPUB INSERT DISPLAY_MODEL

Comment out the NOSTEP line to see the steps correctly.

Expected behaviour

The goal here is to be able to skip showing the step where the BUFEXCHG is retrieved, showing the parts in their final position. Based on this guide: https://sites.google.com/view/workingwithlpub3d/advanced-lessons/buffer-exchange, I believe that is how it is intended to work.

Thanks!

trevorsandy commented 3 years ago

The behaviour displayed above is as designed. In fact, the single-step behaviour is actually how the application should behave because steps containing !LPUB NOSTEP should be completely ignored. For your example above, as you are designating the 0 BUFEXCHG A RETRIEVE step to be !LPUB NOSTEP, the content after 0 BUFEXCHG A STORE is displayed in the following step - step 3.

For multi-step groups, the current behavour will parse and process steps containing !LPUB NOSTEP but these steps' assembly and parts list will not be rendered. Thus, you will have the layout as shown in your third shot where parts after 0 BUFEXCHG A STORE were not rendered.

I will take a look at adding a flag to enable single step page to behave as the multi-step page does for the sake of consistency.

Lastly, there were some inconsistencies in your example file above. I've added comments to highlight them.

0 Name: main.ldr
0 Author: CJ Hendrix
0 // FINAL_MODEL_ENABLED will have no effect if `FADE_STEP` or `HIGHLIGHT_STEP` is not set to TRUE
0 !LPUB FINAL_MODEL_ENABLED GLOBAL FALSE
1 4 -60 40 0 1 0 0 0 1 0 0 0 1 3020.dat
0 STEP
0 BUFEXCHG A STORE
0 !LPUB CALLOUT BEGIN
1 2 -30 -32 0 0 0 -1 0 1 0 1 0 0 3938.dat
1 2 -30 -32 0 0 0 -1 0 1 0 1 0 0 3937.dat
0 // A callout must be a submodel file
0 !LPUB CALLOUT POINTER LEFT 0.298 1.115 0.159 0.000 0.000 0.000 0.000 0.000 0.000 0.000 1
0 !LPUB CALLOUT PLACEMENT RIGHT CENTER ASSEM OUTSIDE OFFSET 0.5695 -0.0388
0 !LPUB CALLOUT END
1 0 -30 40 0 1 0 0 0 1 0 0 0 1 HasvL2.dat
0 STEP
0 !LPUB NOSTEP
0 BUFEXCHG A RETRIEVE
1 2 -30 16 0 0 0 -1 0 1 0 1 0 0 3938.dat
1 2 -30 16 0 0 0 -1 0 1 0 1 0 0 3937.dat
0 STEP
1 1 -60 8 0 1 0 0 0 1 0 0 0 1 3020.dat
0 STEP
0 //DISPLAY_MODEL command must be paired with the !LPUB INSERT PAGE command 
0 !LPUB INSERT PAGE
0 !LPUB INSERT DISPLAY_MODEL

Cheers,

trevorsandy commented 3 years ago

I will take a look at adding a flag to enable single step page to behave as the multi-step page does for the sake of consistency.

Implemented. See #535

Screenshot - 18_04_2021 , 21_35_04

Cheers,

cjhendrix commented 3 years ago

Amazing! Thanks so much.

Sorry about the spurious callout in the sample LDR. I had initially thought having a submodel was required to see the behavior, but later realized it wasn't so I flattened the file, but forgot to delete the callout lines.

Regarding this comment:

0 //DISPLAY_MODEL command must be paired with the !LPUB INSERT PAGE command 
0 !LPUB INSERT PAGE
0 !LPUB INSERT DISPLAY_MODEL

Since version 2.4.2, I've been having trouble with the automatic insertion of the final model corrupting the file. I have several times ended up with copies of a portion of the ldr pasted into the file. I haven't made a ticket yet because I'm not quite sure how to replicate it. But I have tried to disable the automatic insertion of the final model, which does seem to remedy the problem. The comments that are automatically added with the final model commands recommends putting the 0 !LPUB INSERT DISPLAY_MODEL as the last step to override, and doing so seems to prevent the final page and model from being inserted (even though the name of the command seems like it would do the opposite).

Is there a better way to do that (i.e. prevent the final page and model commands from being added automatically)?

If I can figure out a more sensible way of describing the issue with corrupting of the mpd, I'll make a ticket for it. I think it might be related to some unpredictable text selection behavior in the LDraw Editor window.

trevorsandy commented 3 years ago

Thank you for the update.

Since version 2.4.2, I've been having trouble with the automatic insertion of the final model corrupting the file. I have several times ended up with copies of a portion of the ldr pasted into the file.

These are spearate items but, from the other reports I have received, they seem to be occuring in a manner that leads users to believer they are the same. I'll take a look.

Since version 2.4.2, I've been having trouble with the automatic insertion of the final model

The behaviour can be enabled/disabled from the Fade/Highlight global setup dialogue. No other commands should be needed. See the ticket referenced below.

Is there a better way to do that (i.e. prevent the final page and model commands from being added automatically)?

Yes, see #517

The comments that are automatically added with the final model commands recommends putting the 0 !LPUB INSERT DISPLAY_MODEL as the last step to override, and doing so seems to prevent the final page and model from being inserted (even though the name of the command seems like it would do the opposite).

The final model insertion/deletion behaviour will not be triggered if 0 !LPUB INSERT MODEL is not found in the final model step.

The final comments suggest replacing 0 !LPUB INSERT MODEL with 0 !LPUB INSERT DISPLAY_MODEL to disable the automatic insertion(on file load) and deletion (on file save) of the final model commands if you would like to permanently set your final model. To simply disable the final model behaviour, follow the guidance in the ticket referenced above.

I think there was a bug in the official release (2.4.2 r0) and there are some other regressions in the last continuous build so; unfortunately, it would be best to wait for the next update (2.4.3) which should post in the next few days.

If I can figure out a more sensible way of describing the issue with corrupting of the mpd, I'll make a ticket for it. I think it might be related to some unpredictable text selection behavior in the LDraw Editor window.

Correct. This behaviour is linked to efforts to improve the performance of the command editor, notably the paging feature. I thought I treated this some time ago and unset paging as the default configuration. Anyway, I'll take another look.

Cheers,