[Closed] How to simplifly?
Hi, i am a maxscript beginner, seeking for help for now and future to improve and polish my skill here.
on btn1 pressed do
(
if $selection.count> 0 then
(
for i=1 to $selection.count do
(
blah blah
)
)
else
(
messagebox "Please select an object to proceed."
)
)
on btn2 pressed do
(
if $selection.count> 0 then
(
for i=1 to $selection.count do
(
blah blah blah
)
)
else
(
messagebox "Please select an object to proceed."
)
)
on btn3 pressed do
(
if $selection.count> 0 then
(
for i=1 to $selection.count do
(
blah blah
)
)
else
(
messagebox "Please select an to proceed."
)
)
Actually i have ten plus button like this in my script, how can i simplify it?
Thanks in advance
first of all if it’s your first code to review (mxs question, etc.) you are welcome.
this forum is the best MXS community IMHO.
the second is – if you post your code please put it in-between <code> tag. it’s easy. just select your code and press # button in the message edit window
what is the difference between –
$
$selection
selection
?
they are all almost about the same – the current selection
but… everything that starts from ‘$’ means “value path”.
and depends on state it can be a node or node collection, or nothing (undefined)
so…
$
- undefined if nothing is selected
- a node if only one node selected
- a node collection if multiple node selection
$selection
- always a collection of selected nodes (but at the same time it might be a root of value path)
selection
- it’s exact node collection of selected nodes
in your case i would definitely use ‘selection’
so it would be like:
on <button> pressed do
(
blah selection
)
but because usually ‘blah’ doesn’t do anything smart with an empty collection i do it as:
on <button> pressed do if selection.count > 0 do
(
blah selection
)
why?
easy… in other case i have to add ‘a check of emptiness’ in the ‘blah’ function i call. it slows down the ‘blah’ function
Sorry, i have edit my message, hoping i am doing correct now
thank you for your explain, hmmm i am very new, trying to figure out what are you trying to tell me and change my code as below:
on btn1 pressed do
(
if selection.count> 0 then
(
for i=1 to selection.count do
(
blah blah
)
)
else
(
messagebox "Please select an object to proceed."
)
)
Is this correct?
that’s correct according to my coding conventions
but if we are talking about real code and my preferences it should be:
on btn1 pressed do
(
if selection.count> 0 then
(
for node in selection do
(
blah node
)
)
else ...
)
if a collection is iteratable i would use “for x in” contraction instead of an iterating by index (for k=1 to …count)
i have change and test code according to your guide, it’s work fine at this moment.
here is my original code:
on kwb_btn8 pressed do
(
if selection.count> 0 then
(
for i=1 to selection.count do
(
$.whiteBalance_preset = 4
)
)
else
(
messagebox "Please select a VrayPhysicalCamera to proceed." title:"VrayPhysicalCamera"
)
)
actually i have more than 10 one click button like this in a single script, i will only change this “$.whiteBalance_preset = 4” to preset =1, preset=2 and so on…
my question is, am i have to type the below code to make the button
else
(
messagebox "Please select a VrayPhysicalCamera to proceed." title:"VrayPhysicalCamera"
)
or is there any way to simplify this?
i would like to show my rollout to make you clear what am i trying to do.
all button you see in the image will give you a warning and ask you to select vrayphysicalcamera if you didn’t select any
Well you’ve got two ways of doing it, you either restrict the user to having to filter their selection to only Vray Cameras or you let your code do that for you.
At the moment your code isn’t even checking that your selection is in fact a VrayPhysicalCamera…
You can do it like this…
if selection.count != 0 and classof selection[1] == VRayPhysicalCamera do
or like this:
for o in selection where classof o == VrayPhysicalCamera do o.whitebalance_preset = 4
or if you know you want to do it to all VrayCameras in the scene you can do this…
for o in (getclassinstances VRayPhysicalCamera) do o.whitebalance_preset = 4
This section in your code is currently not doing quite what you probably expect it to…
for i=1 to selection.count do
(
$.whiteBalance_preset = 4
)
$.whiteBalance_preset = 4 will change that value on the selected object(s)… try it using the maxscript listener, select a bunch of cameras and then just run that one line, you’ll see that it updates it on every camera in one go, that’s because the objects are the same class, maxscript knows to do it to all of them, but because you’re running this in a loop of the selection length you are changing them all to 4 in every iteration in the loop.
I think it’s better practice to use the index of the your selection in your loops… eg…
for i=1 to selection.count do
(
selection[i].whiteBalance_preset = 4
)
Hi DaveWortley,
your website is in my bookmark list too, nice to see you here
since my english is poor, i am hard to understand and sometime confuse when use google translate to translate it to chinese, so i decide to write some script and find a place to post it and learn.
If you look into the image which i post in previous post and compare the script below, i hope you will get what i want my script to do.
i am trying to click a simple vraycamera setting tool for my team to use it in office daily work, the idea is i am trying to make a one click preset button for them, but if they didn’t select anything and click the button, the script will give them a warning and pop up a message ask them to select at lease one vraycamera to use the script…
the script work fine in my office, but i want to learn to become a proper script write, so i post it here.
anyway, i know i still got a long long way to go after i post my script here, now i know there are a lot of way to make a script selection (headache now…)
on btn1 pressed do
(
if selection.count> 0 then
(
for x in selection do
(
x.f_number = 8
x.shutter_speed = 60
x.ISO = 100
x.vignetting = off
x.whiteBalance_preset = 1
)
)
else
(
messagebox "Please select a VrayPhysicalCamera to proceed." title:"VrayPhysicalCamera"
)
)
on btn2 pressed do
(
if selection.count> 0 then
(
for x in selection do
(
x.f_number = 8
x.shutter_speed = 500
x.ISO = 100
x.vignetting = off
x.whiteBalance_preset = 7
x.temperature = 4000
)
)
else
(
messagebox "Please select a VrayPhysicalCamera to proceed." title:"VrayPhysicalCamera"
)
)
on btn1 pressed do
(
for i=1 to selection.count do
(
selection[i].whiteBalance_preset = 4
)
else
(
messagebox "Please select a VrayPhysicalCamera to proceed." title:"VrayPhysicalCamera"
)
)
i put the code like this, and i get error message like this:
-- Error occurred in anonymous codeblock; filename: E:\maxscript\maxscript\MyScript\VRCam Preset v1.1_test.ms; position: 328; line: 19
-- Syntax error: at else, expected <factor>
-- In line: else
You recive error because you not have right “if-then-else” statement. Try like this
on btn1 pressed do
(
for i=1 to selection.count do
(
if classof selection[i] == VrayPhysicalCamera then
(
selection[i].whiteBalance_preset = 4
)
else
(
messagebox "Please select a VrayPhysicalCamera to proceed." title:"VrayPhysicalCamera"
)
)
Use this form instead
on btn1 pressed do if selection.count != 0 do
(
for i = 1 to selection.count where isKindOf (camera = selection[i]) VrayPhysicalCamera do
(
camera.f_number = 8
camera.shutter_speed = 60
camera.ISO = 100
camera.vignetting = off
camera.whiteBalance_preset = 1
camera.whiteBalance_preset = 4
)
)
camera (like node) is predefined variable but U can use simple word or letter (like “x”) as variable in this case
it seems like the button btn1 sets some settings for all or selected VrayPhysicalCameras.
it would be better:
#1 group all properties and default values as a preset. it will help you to be able easy extend the list
#2 set all properties off all cameras at the same time. in this case you have to filter them first
#3 extend functionality of the button to be able set all cameras instead of selected only
#4 make operation undoable
fn findVrayPhysicalCameras selected:off =
(
cams = getclassinstances VrayPhysicalCamera
if selected do cams = for c in cams where c.isselected collect c
cams
)
local preset1 =
#(
#(#f_number, 8),
#(#shutter_speed, 60),
#(#ISO, 100),
#(#vignetting, off),
#(#whiteBalance_preset, 1),
#(#whiteBalance_preset, 4)
)
on btn1 pressed do undo "Change Vray Physical Cameras" on
(
all_cameras = keyboard.controlpressed
cams = findVrayPhysicalCameras selected:(not all_cameras)
if cams.count > 0 do
(
for p in preset1 do setproperty cams p[1] p[2]
)
)