Merge pull request #7878 from RebelliousX/else

Bunch of missing `else` statements and general logic

core/io/http_client.cpp

@@ -340,7 +340,7 @@ Error HTTPClient::poll(){
 				int rs = response_str.size();
 				if (
 					(rs>=2 && response_str[rs-2]=='\n' && response_str[rs-1]=='\n') ||
-					(rs>=4 && response_str[rs-4]=='\r' && response_str[rs-3]=='\n' && rs>=4 && response_str[rs-2]=='\r' && response_str[rs-1]=='\n')
+					(rs>=4 && response_str[rs-4]=='\r' && response_str[rs-3]=='\n' && response_str[rs-2]=='\r' && response_str[rs-1]=='\n')
 				) {
 
 

core/io/marshalls.cpp

@@ -641,7 +641,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
 
 			ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
 			uint32_t count = decode_uint32(buf);
-			ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
 
 			PoolVector<String> strings;
 			buf+=4;
@@ -691,7 +690,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
 
 			ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
 			uint32_t count = decode_uint32(buf);
-			ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
 			buf+=4;
 			len-=4;
 
@@ -729,7 +727,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
 
 			ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
 			uint32_t count = decode_uint32(buf);
-			ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
 			buf+=4;
 			len-=4;
 
@@ -768,7 +765,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
 
 			ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
 			uint32_t count = decode_uint32(buf);
-			ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
 			buf+=4;
 			len-=4;
 

core/io/stream_peer.cpp

@@ -492,6 +492,8 @@ Error StreamPeerBuffer::get_partial_data(uint8_t* p_buffer, int p_bytes,int &r_r
 
 	PoolVector<uint8_t>::Read r = data.read();
 	copymem(p_buffer,r.ptr(),r_received);
+	
+	// FIXME: return what? OK or ERR_*
 }
 
 int StreamPeerBuffer::get_available_bytes() const {

core/pool_allocator.cpp

@@ -504,7 +504,7 @@ const void *PoolAllocator::get(ID p_mem) const {
 		return NULL;
 	}
 
-	if (e->pos<0 || (int)e->pos>=pool_size) {
+	if ((int)e->pos>=pool_size) {
 
 		mt_unlock();
 		ERR_PRINT("e->pos<0 || e->pos>=pool_size");
@@ -546,7 +546,7 @@ void *PoolAllocator::get(ID p_mem) {
 		return NULL;
 	}
 
-	if (e->pos<0 || (int)e->pos>=pool_size) {
+	if ((int)e->pos>=pool_size) {
 
 		mt_unlock();
 		ERR_PRINT("e->pos<0 || e->pos>=pool_size");

core/variant.cpp

@@ -2953,7 +2953,7 @@ uint32_t Variant::hash() const {
 		PoolVector<p_type>::Read rr = r.read(); \
 		\
 		for(int i = 0; i < l.size(); ++i) { \
-			if(! p_compare_func((lr[0]), (rr[0]))) \
+			if(! p_compare_func((lr[i]), (rr[i]))) \
 				return false; \
 		}\
 		\
@@ -3065,7 +3065,7 @@ bool Variant::hash_compare(const Variant& p_variant) const {
 				return false;
 
 			for(int i = 0; i < l.size(); ++i) {
-				if(! l[0].hash_compare(r[0]))
+				if(! l[i].hash_compare(r[i]))
 					return false;
 			}
 

core/variant_op.cpp

@@ -1393,7 +1393,7 @@ void Variant::set(const Variant& p_index, const Variant& p_value, bool *r_valid)
 						v->basis.set_axis(index,p_value);
 					return;
 				}
-			} if (p_index.get_type()==Variant::STRING) {
+			} else if (p_index.get_type()==Variant::STRING) {
 
 				Transform *v=_data._transform;
 				const String *str=reinterpret_cast<const String*>(p_index._data._mem);
@@ -2150,7 +2150,7 @@ Variant Variant::get(const Variant& p_index, bool *r_valid) const {
 					valid=true;
 					return index==3?v->origin:v->basis.get_axis(index);
 				}
-			} if (p_index.get_type()==Variant::STRING) {
+			} else if (p_index.get_type()==Variant::STRING) {
 
 				const Transform *v=_data._transform;
 				const String *str=reinterpret_cast<const String*>(p_index._data._mem);

core/variant_parser.cpp

@@ -1414,7 +1414,7 @@ Error VariantParser::parse_value(Token& token,Variant &value,Stream *p_stream,in
 			return OK;
 		} else if (id=="img") {  // compatibility with godot.cfg
 
-			Token token;
+			Token token;	// FIXME: no need for this declaration? the first argument in line 509 is a Token& token.
 			get_token(p_stream,token,line,r_err_str);
 			if (token.type!=TK_PARENTHESIS_OPEN) {
 				r_err_str="Expected '(' in old-style godot.cfg construct";

drivers/gles3/rasterizer_canvas_gles3.cpp

@@ -1010,7 +1010,7 @@ void RasterizerCanvasGLES3::canvas_render_items(Item *p_item_list,int p_z,const
 		if (unshaded || (state.canvas_item_modulate.a>0.001 && (!shader_cache || shader_cache->canvas_item.light_mode!=RasterizerStorageGLES3::Shader::CanvasItem::LIGHT_MODE_LIGHT_ONLY) && !ci->light_masked ))
 			_canvas_item_render_commands(ci,current_clip,reclip);
 
-		if ((blend_mode==RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_MIX || RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_PMALPHA) && p_light && !unshaded) {
+		if ((blend_mode==RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_MIX || blend_mode==RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_PMALPHA) && p_light && !unshaded) {
 
 			Light *light = p_light;
 			bool light_used=false;

drivers/gles3/rasterizer_scene_gles3.cpp

@@ -2806,6 +2806,7 @@ void RasterizerSceneGLES3::_copy_to_front_buffer(Environment *env) {
 		//no environment, simply convert from linear to srgb
 		storage->shaders.copy.set_conditional(CopyShaderGLES3::LINEAR_TO_SRGB,true);
 	} else {
+		/* Why are both statements equal? */
 		storage->shaders.copy.set_conditional(CopyShaderGLES3::LINEAR_TO_SRGB,true);
 
 	}

drivers/gles3/shader_compiler_gles3.cpp

@@ -401,6 +401,7 @@ String ShaderCompilerGLES3::_dump_node_code(SL::Node *p_node, int p_level, Gener
 				String scode = _dump_node_code(bnode->statements[i],p_level,r_gen_code,p_actions,p_default_actions);
 
 				if (bnode->statements[i]->type==SL::Node::TYPE_CONTROL_FLOW || bnode->statements[i]->type==SL::Node::TYPE_CONTROL_FLOW) {
+					// FIXME: if (A || A) ? I am hesitant to delete one of them, could be copy-paste error.
 					code+=scode; //use directly
 				} else {
 					code+=_mktab(p_level)+scode+";\n";

main/tests/test_math.cpp

@@ -144,11 +144,11 @@ class GetClassAndNamespace {
 									error_str="Unterminated comment";
 									error=true;
 									return TK_ERROR;
-								} if (code[idx]=='*' &&code[idx+1]=='/') {
+								} else if (code[idx]=='*' &&code[idx+1]=='/') {
 
 									idx+=2;
 									break;
-								} if (code[idx]=='\n') {
+								} else if (code[idx]=='\n') {
 									line++;
 								}
 

modules/gdscript/gd_function.cpp

@@ -725,7 +725,7 @@ Variant GDFunction::call(GDInstance *p_instance, const Variant **p_args, int p_a
 								err.argument-=1;
 							}
 						}
-					} if (methodstr=="free") {
+					} else if (methodstr=="free") {
 
 						if (err.error==Variant::CallError::CALL_ERROR_INVALID_METHOD) {
 

modules/regex/regex.cpp

@@ -321,7 +321,7 @@ struct RegExNodeClass : public RegExNode {
 			case Type_lower:
 				return ('a' <= c && c <= 'z');
 			case Type_print:
-				return (0x1F < c && c < 0x1F);
+				return (0x20 < c && c < 0x7f);
 			case Type_punct:
 				return (REGEX_NODE_PUNCT.find(c) >= 0);
 			case Type_space:

modules/visual_script/visual_script_builtin_funcs.cpp

@@ -329,7 +329,7 @@ PropertyInfo VisualScriptBuiltinFunc::get_input_value_port_info(int p_idx) const
 		case LOGIC_CLAMP: {
 			if (p_idx==0)
 				return PropertyInfo(Variant::REAL,"a");
-			else if (p_idx==0)
+			else if (p_idx==0)	// is it ok to test p_idx == 0 twice?
 				return PropertyInfo(Variant::REAL,"min");
 			else
 				return PropertyInfo(Variant::REAL,"max");

scene/2d/particles_2d.cpp

@@ -229,6 +229,7 @@ ParticleAttractor2D::ParticleAttractor2D() {
 /****************************************/
 
 _FORCE_INLINE_ static float _rand_from_seed(uint64_t *seed) {
+
 	uint32_t r = Math::rand_from_seed(seed);
 	return 2.0f * (float)r / (float)Math::RANDOM_MAX - 1.0f;
 }

scene/gui/control.cpp

@@ -2421,8 +2421,6 @@ void Control::get_argument_options(const StringName& p_function,int p_idx,List<S
 			Theme::get_default()->get_font_list(get_class(),&sn);
 		} else if (pf=="add_constant_override" || pf=="has_constant" || pf=="has_constant_override" || pf=="get_constant") {
 			Theme::get_default()->get_constant_list(get_class(),&sn);
-		} else if (pf=="add_color_override" || pf=="has_color" || pf=="has_color_override" || pf=="get_color") {
-			Theme::get_default()->get_color_list(get_class(),&sn);
 		}
 
 		sn.sort_custom<StringName::AlphCompare>();

scene/gui/dialogs.cpp

@@ -201,7 +201,7 @@ void AcceptDialog::_notification(int p_what) {
 	if (p_what==NOTIFICATION_MODAL_CLOSE) {
 
 		cancel_pressed();
-	} if (p_what==NOTIFICATION_RESIZED) {
+	} else if (p_what==NOTIFICATION_RESIZED) {
 
 		_update_child_rects();
 	}

scene/gui/item_list.cpp

@@ -314,7 +314,7 @@ void ItemList::move_item(int p_item,int p_to_pos) {
 
 	if (current<0) {
 		//do none
-	} if (p_item==current) {
+	} else if (p_item==current) {
 		current=p_to_pos;
 	} else if (p_to_pos>p_item && current>p_item && current<p_to_pos) {
 		current--;

scene/gui/rich_text_label.h

@@ -173,7 +173,7 @@ private:
 
 	struct ItemNewline : public Item {
 
-		int line;
+		int line;	// FIXME: Overriding base's line ?
 		ItemNewline() { type=ITEM_NEWLINE; }
 	};
 

scene/gui/text_edit.cpp

@@ -4267,7 +4267,7 @@ void TextEdit::_update_completion_candidates() {
 		//no completion here
 		//print_line("cancel!");
 		cancel=true;
-	} if (inquote && first_quote!=-1) {
+	} else if (inquote && first_quote!=-1) {
 
 		s=l.substr(first_quote,cofs-first_quote);
 		//print_line("s: 1"+s);

scene/resources/default_theme/default_theme.cpp

@@ -986,7 +986,7 @@ void make_default_theme(bool p_hidpi,Ref<Font> p_font) {
 	Ref<BitmapFont> default_font;
 	if (p_font.is_valid()) {
 		default_font=p_font;
-	} if (p_hidpi) {
+	} else if (p_hidpi) {
 		default_font=make_font2(_hidpi_font_height,_hidpi_font_ascent,_hidpi_font_charcount,&_hidpi_font_charrects[0][0],_hidpi_font_kerning_pair_count,&_hidpi_font_kerning_pairs[0][0],_hidpi_font_img_width,_hidpi_font_img_height,_hidpi_font_img_data);
 	} else {
 		default_font=make_font2(_lodpi_font_height,_lodpi_font_ascent,_lodpi_font_charcount,&_lodpi_font_charrects[0][0],_lodpi_font_kerning_pair_count,&_lodpi_font_kerning_pairs[0][0],_lodpi_font_img_width,_lodpi_font_img_height,_lodpi_font_img_data);

servers/physics/shape_sw.cpp

@@ -181,7 +181,7 @@ void RayShapeSW::get_supports(const Vector3& p_normal,int p_max,Vector3 *r_suppo
 		r_amount=2;
 		r_supports[0]=Vector3(0,0,0);
 		r_supports[1]=Vector3(0,0,length);
-	} if (p_normal.z>0) {
+	} else if (p_normal.z>0) {
 		r_amount=1;
 		*r_supports=Vector3(0,0,length);
 	} else {

servers/visual/shader_language.cpp

@@ -307,7 +307,7 @@ ShaderLanguage::Token ShaderLanguage::_get_token() {
 							} if (GETCHAR(0)=='*' && GETCHAR(1)=='/') {
 								char_idx+=2;
 								break;
-							} if (GETCHAR(0)=='\n') {
+							} else if (GETCHAR(0)=='\n') {
 								tk_line++;
 							}
 
@@ -3241,7 +3241,7 @@ Error ShaderLanguage::_parse_shader(const Map< StringName, Map<StringName,DataTy
 					_set_error("void datatype not allowed here");
 					return ERR_PARSE_ERROR;
 				}
-				if (!uniform && type<TYPE_FLOAT && type>TYPE_VEC4) {
+				if (!uniform && type<TYPE_FLOAT && type>TYPE_VEC4) {	// FIXME: always false! should it be || instead?
 					_set_error("Invalid type for varying, only float,vec2,vec3,vec4 allowed.");
 					return ERR_PARSE_ERROR;
 				}

tools/editor/collada/collada.cpp

@@ -352,7 +352,7 @@ void Collada::_parse_image(XMLParser& parser) {
 
 					image.path=path;
 
-				} if (name=="data") {
+				} else if (name=="data") {
 
 					ERR_PRINT("COLLADA Embedded image data not supported!");
 
@@ -728,7 +728,7 @@ void Collada::_parse_effect_material(XMLParser& parser,Effect &effect,String &id
 #endif
 
 						}
-					} if (parser.get_node_type() == XMLParser::NODE_ELEMENT_END && (
+					} else if (parser.get_node_type() == XMLParser::NODE_ELEMENT_END && (
 						parser.get_node_name()=="constant" ||
 						parser.get_node_name()=="lambert" ||
 						parser.get_node_name()=="phong"  ||
@@ -1100,7 +1100,7 @@ void Collada::_parse_mesh_geometry(XMLParser& parser,String p_id,String p_name)
 				current_source=id;
 				COLLADA_PRINT("source data: "+id);
 
-			} else if (section=="float_array" || section=="array" || section=="float_array") {
+			} else if (section=="float_array" || section=="array") {
 				// create a new array and read it.
 				if (meshdata.sources.has(current_source)) {
 
@@ -2192,7 +2192,7 @@ void Collada::_parse_scene(XMLParser& parser) {
 
 				state.root_visual_scene=_uri_to_id(parser.get_attribute_value("url"));
 				print_line("***ROOT VISUAL SCENE: "+state.root_visual_scene);
-			} if (name=="instance_physics_scene") {
+			} else if (name=="instance_physics_scene") {
 
 				state.root_physics_scene=_uri_to_id(parser.get_attribute_value("url"));
 

tools/editor/import/resource_importer_obj.cpp

@@ -118,7 +118,7 @@ Error ResourceImporterOBJ::import(const String& p_source_file, const String& p_s
 			nrm.y=v[2].to_float();
 			nrm.z=v[3].to_float();
 			normals.push_back(nrm);
-		} if (l.begins_with("f ")) {
+		} else if (l.begins_with("f ")) {
 				//vertex
 
 			has_index_data=true;

tools/editor/import/resource_importer_scene.cpp

@@ -1279,7 +1279,7 @@ Error ResourceImporterScene::import(const String& p_source_file, const String& p
 	Ref<EditorScenePostImport>  post_import_script;
 
 	if (post_import_script_path!="") {
-		post_import_script_path = post_import_script_path;
+		post_import_script_path = post_import_script_path;	// FIXME: is there a good reason for this?
 		Ref<Script> scr = ResourceLoader::load(post_import_script_path);
 		if (!scr.is_valid()) {
 			EditorNode::add_io_error(TTR("Couldn't load post-import script:")+" "+post_import_script_path);

tools/editor/plugins/collision_polygon_2d_editor_plugin.cpp

@@ -268,7 +268,7 @@ bool CollisionPolygon2DEditor::forward_gui_input(const InputEvent& p_event) {
 								return true;
 							}
 						}
-					} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
+					} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
 
 
 

tools/editor/plugins/light_occluder_2d_editor_plugin.cpp

@@ -283,7 +283,7 @@ bool LightOccluder2DEditor::forward_gui_input(const InputEvent& p_event) {
 								return true;
 							}
 						}
-					} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
+					} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
 
 
 

tools/editor/plugins/navigation_polygon_editor_plugin.cpp

@@ -312,7 +312,7 @@ bool NavigationPolygonEditor::forward_gui_input(const InputEvent& p_event) {
 								return true;
 							}
 						}
-					} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
+					} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
 
 						int closest_outline=-1;
 						int closest_idx=-1;

tools/editor/plugins/polygon_2d_editor_plugin.cpp

@@ -385,7 +385,7 @@ bool Polygon2DEditor::forward_gui_input(const InputEvent& p_event) {
 								return true;
 							}
 						}
-					} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
+					} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
 
 
 

tools/editor/plugins/texture_region_editor_plugin.cpp

@@ -353,6 +353,7 @@ void TextureRegionEditor::_region_input(const InputEvent& p_input)
 						undo_redo->add_undo_method(atlas_tex.ptr(),"set_region",rect_prev);
 					}
 					else if(node_patch9){
+						// FIXME: Is this intentional?
 					} else if(node_patch9){
 						undo_redo->add_do_method(node_patch9 ,"set_region_rect",node_patch9->get_region_rect());
 						undo_redo->add_undo_method(node_patch9,"set_region_rect",rect_prev);

tools/editor/property_editor.cpp

@@ -534,7 +534,7 @@ bool CustomPropertyEditor::edit(Object* p_owner,const String& p_name,Variant::Ty
 
 			} else if (hint==PROPERTY_HINT_TYPE_STRING) {
 
-
+				/* FIXME: This is repeated twice, with slightly different behavior! Which one? Check line 644 */
 				if (!create_dialog) {
 					create_dialog = memnew( CreateDialog );
 					create_dialog->connect("create",this,"_create_dialog_callback");
@@ -651,6 +651,7 @@ bool CustomPropertyEditor::edit(Object* p_owner,const String& p_name,Variant::Ty
 
 			} else if (hint==PROPERTY_HINT_TYPE_STRING) {
 				if (!create_dialog) {
+					/* FIXME: ... and here. See line 529 */
 					create_dialog = memnew( CreateDialog );
 					create_dialog->connect("create",this,"_create_dialog_callback");
 					add_child(create_dialog);
@@ -2384,6 +2385,7 @@ void PropertyEditor::set_item_text(TreeItem *p_item, int p_type, const String& p
 				p_item->set_range(1, obj->get( p_name ) );
 
 			} else {
+				/* FIXME: Why are both statements equal? */
 				p_item->set_range(1, obj->get( p_name ) );
 			}