After doing some cleanup the patches are ready for review and (hopefully) committing to cvs again.
The new versions are here: http://www.akermann.org/fgfs/halos.tgz http://www.akermann.org/fgfs/flightgear-sun-2006072201.tgz http://www.akermann.org/fgfs/simgear-sun-2006072201.tgz I tried to do some changes on the halo-texture but there was no difference, so I'll leave the old one in the package for now. The code was cleaned and polished and I must admit it wasn't very consistent. Since there are many people involved in this project it's necessary to keep to certain conventions.-) I didn't touch the vector calculations as Vassilii suggested though. OK, still awaiting comments and suggestions, Mark BTW: Is it better to have one big file with all diffs concatenated or to provide a file for every file changed? Melchior FRANZ wrote: > * Mark -- Thursday 20 July 2006 20:31: > >> After it is in cvs I'd be happy for some feedback and suggestions for >> improvement. >> > > Here's some already, mostly formal problems. (I'm the pedant here.) > The important aspects seem to be OK. > > > fgGetNode("/environment")); > > ... in a constructor. Although the existence of the "/environment" is > pretty much guaranteed, we still make sure that the node gets created: > > fgGetNode("/environment", true)); > ^^^^ > > > ---------------------------------------------------------------------- > consistency: > > + SGPath ihalopath = path , ohalopath=path; > > Now, what is it? spaces around '=' (good) or not? Space in front of > comma operator? > > + i_halo_color[0] = 1- (1.1* red_scat_f); > + o_halo_color[0] = 1- (1.4* red_scat_f); > > Operators sticking on numbers at several places. Either spaces around > operators (good IMHO) or not. Or is they kind of unary operators? :-} > > > ---------------------------------------------------------------------- > > > indentation: > > + ohalo->setCallback( SSG_CALLBACK_PREDRAW, sgSunHaloPreDraw ); > + ohalo->setCallback( SSG_CALLBACK_POSTDRAW, sgSunHaloPostDraw ); > + > + sun_transform->addKid( ohalo ); > + sun_transform->addKid( ihalo ); > > What about aligning those correctly? > > > ---------------------------------------------------------------------- > > > + if ( env_node ){ > + env_node->setDoubleValue("atmosphere/altitude-troposphere-top",r_tropo > - r_earth); > + env_node->setDoubleValue("atmosphere/altitude-half-to-sun", alt_half); > + } > > Umm ... what about indenting that correctly? > Yes, I know, the existing code isn't consistent either. But that's no > reason to make the situation worse. > > m. > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Flightgear-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/flightgear-devel > ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Flightgear-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/flightgear-devel
