text:
(Cosmetic) (Minor) UI Elements, Leak Out or Overlap on Each Other, When Using, Non Standard, Font Sizes

Godot 3.1.1
On the Top Left. Select Editor, Editor Settings.
Under Interface, Select Editor.
Set the Main Font Size to 16.
https://i.imgur.com/8RwSYxF.png
https://i.imgur.com/wxOkaZN.png
Font Size 20
https://i.imgur.com/R7Po11j.png
https://i.imgur.com/zPgY5Bk.png
https://i.imgur.com/Mnp2JtK.png
https://i.imgur.com/zGm8KoO.png
https://i.imgur.com/ztuDE2f.png
https://i.imgur.com/uTKSBg1.png
https://i.imgur.com/pbuwM8k.png
I think this is confusing behaviour for new users, I've never used Godot, and to me it seemed like the UI was broken at first, as I'm used to most user interfaces fixing themselves, without having to restart the whole program.
I didn't find restarting Godot to be intuitive, and while I did figure out what was going on, I think this problem should be adressed.
Even the most minor of issues, can put off new users from using the engine in my mind.
I think this message lacks clarity, as I wasn't really sure what it really ment, until I restarted the whole engine.
https://i.imgur.com/OMVUvGW.png
I think this should be fixed, it's janky as hell, and even experienced users once in a while can get confused by weird one time behaviours, and I really think that, such visible problems shouldn't be there. Anyway, ideally just, update the UI, without forcing a restart, or something, I don't know.
I know that a lot of people are gonna complain that, this is such a minor problem, that's not even worth bringing up, but I really want to point out such issues, as a new user, because more often than not, these "minor" problems add up, and they all start to pile up in the future, and it can have a larger impact on everything else.
I also think it's a very good idea, to provide an enjoyable, and intuitive first time user experience, with the least amount of jank possible. I think the engine should be fun and enjoyable to use at it's core.
I'm not gonna be able to use Godot for the first time ever again, and I hope people don't underestimate such feedback. I hope people won't dismiss this, just because it seems like a "minor" or "not important" problem at a first glance.
I would fix this myself, but I'm not experienced enough, sorry.

While I wrote that, I actually had a small idea, as to how to help users out.
Just add one extra warning, if you detected that the font sizes have been changed.

This would only be a temporary thing, but at least there would be some feedback.

Hi @RaTcHeT302! If you have time, please reduce your issue, it will help a lot. To my understanding, it seems like when modifying the font size for the Editor, some UI elements are not updated correctly.

Changing the message to fit specific settings would require a lot of work compared to the gain.

as I'm used to most user interfaces fixing themselves, without having to restart the whole program.

Like what? If I change themes or similar settings on Windows or Linux, I fully expect to have to restart some apps or even the whole system to apply the changes. I don't see the issue with requiring users to restart the editor to correctly apply these changes.

Closing, as there is already a hint to restart the editor after changing the font size.

authors: Calinou RaTcHeT302 aaronfranke ghost girng


text:
Typos in method/constant names

Here I suppose it means SUBTRACT instead?

get_video_texture* ?
I was fixing typos but feared that changing those is not a good idea so I made an issue instead.

Nice catch :) I was dead convinced that "substract" is correct, but actually it's the ancient form of "subtract" :D
get_video_texutre should indeed be get_video_texture.
You can't fix those directly in classes.xml, they need to be fixed in the source code, and then the doc file regenerated with -doctool. It will break the API, but at least for get_video_texutre I think it's fine, if anyone had been using it, they would have reported the typo :p

I think I'll try to get into the code and try to fix it :D
as for the 'substract' are you saying that it should remain unchanged?

as for the 'substract' are you saying that it should remain unchanged?

I think it could be fixed too, unless @reduz objects to it as it would break compatibility for users that rely on KEY_KP_SUBSTRACT. It's still quite easy for them to check the Global Scope and find the new name :)

At least KEY_KB_SUBTRACT can be added as an alias and the typo be marked as deprecated to be removed in the next version.

source: gharchive/issue

authors: akien-mga lordadamson vnen

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
Editor Settings > Run > Window Placement > Screen: Doesn't Work

Godot version:
v3.2.stable.official
OS/device including version:
macOS Catalina Version 10.15.2
Issue description:
I want to use the "Editor Settings > Run > Window Placement > Screen" setting to make Godot run my game on a specific screen, specifically my laptop. However, no matter what I change the setting to, the game runs on my external monitor.
Steps to reproduce:

Use external monitor.
Change Editor Settings > Run > Window Placement > Screen to any setting.
Game always runs on external monitor, not laptop, when using Cmd + B shortcut.

Minimal reproduction project:
Any project with one scene.

I can confirm this on commit https://github.com/godotengine/godot/commit/9a55f1564c96baff19c5595aa3e1ea296f9bd260 (Fedora 31 KDE). This only occurs when the project is configured to use fullscreen, though.

Can anyone still reproduce this bug in Godot 3.2.3 or any later release?
Looks like opposite of #20618

Godot v3.3.4.
Running on second monitor ONLY works when the Project Settings is configured to run in a window, but the Editor Settings Window Placement options are configured to force fullscreen.
Any other configurations don't seem to work.
Might be worth noting that my two monitors have different resolutions (Monitor 1 is 4k and monitor 2 is HD).

I'm seeing this in Godot v4.0 alpha 6. Strangely had no issue in Godot v4.0 alpha 4 maybe something recently merged to master with regards to window placement.

Seems like no one was able to reproduce the original issue, so closing as duplicate of #21746 (i.e. only broken in fullscreen mode)

Workaround is to disable / enable project settings depending on whether you are working in editor or doing a build sweat_smile

You can also use the editor or standalone feature tag override for this.

This is still a problem in 4.0 stable.
It's indeed a duplicate of #20618, but that issue was closed as duplicate of another issue, which also was closed as duplicate, of an issue that was fixed. The original issue is still relevant though, so mine wasn't a duplicate in the end.

I see something that sounds identical, today, v4.2.1 stable.

I may be holding something wrong, but I can't run the project on a different screen than the editor window.
I've tried every single option for run/window_placement/screen.

Godot v4.2.1 on Windows 10 - It worked fine but suddenly the project starts only on primary monitor and the project settings is ignored.

Window Placement is missing entirely in 4.3. Looks like this whole setting feature is being phased out.

I solved it by putting the Viewport Width and Height lower than the screen I wanted to run the game on.
Project Settings -> Display -> Window
Now it correctly opens on the screen set in Editor -> Run -> Window Placement

authors: Calinou EmilPastelka Jelligeth KoBeWi Luke9389 arcticmatt aristides86 joemicmc kristianpiacenti tartley


text:
"animation_finished" Missing From AnimatedSprite3D

Godot version:
3.2.2
OS/device including version:
Manjaro Linux x86_64
Kernel 5.4.44-1-MANJARO
Issue description:
There is no animation_finished signal for AnimatedSprite3D unlike AnimatedSprite.
As the other signal, frame_changed, is present I assume this is either intentional or an accidental omission as they don't share any relevant common ancestor in their inheritance hierarchy.
Steps to reproduce:

Open the editor.
Create an AnimatedSprite3D node.
Select it and navigate to its signals tab.

Minimal reproduction project:
I do not think one is necessary doesn't require any project setup, however if requested I can provide one.

I can take this if no one else has started yet

I met the same problem with this and also with frame_changed not fired. Please, merge the fix provided.

authors: Kayomn jiteshvm seltee


text:
Window Dialogs Don't Show

Operating system or device:
Linux
Issue description (what happened, and what was expected):
Just last night I got Window Dialogs to show, but today they won't. Only thing that has changed is that I moved the Godot executable to /usr/bin last night. Today I opened the project and the windows don't show when running. They show in edit mode, though. Clicking the visibility (eye icon) does nothing to show it. I have to add a script to the window and add..
func _init():      show()
for it to show. (show() should be indented on the next line in that statement but I couldn't get it to work on github.
WindowTest.tar.gz
Steps to reproduce:
Create a Window Dialog, any kind and it won't show on my system.
Link to minimal example project (optional but very welcome):

WindowDialog is an inheritor of_Popup_, so you must use either popup(), popup_centered( Vector2 size=Vector2(0,0) ), popup_centered_ratio( float ratio=0.75 ) or popup_centered_minsize( Vector2 minsize=Vector2(0,0) ) to show it.

I'm wondering why it worked last night. I had it as a child of a control node. No popup nodes involved at all.

I'm not quite sure what you're telling me to do. I need to dig deep into Godot again. I made WindowDialog a child of Popup, which is a child of Control and it still won't show.

I mean WindowDialog is a derived class of Popup (check the WindowDialog reference).
You must call popup() (which is a method from the Popup class) to show it in a modal form. If you want to show your dialog automatically when starting, you have two options:

Add a script to the WindowDialog, override _ready() method and call popup() there:

extends WindowDialog

func _ready():
    popup()
    # or popup_centered()
    # or popup_centered_minsize()
    # or popup_centered_ratio()

You can also do it from the editor by connecting the signal enter_tree with the popup method in deferred mode, like explained here: https://github.com/godotengine/godot/issues/2431#issuecomment-198761429

Thanks, I think I understand. I just don't get why it worked automatically last night and not today. Either way I have a lot of learning to do. This has inspired me to redouble my efforts.

authors: neikeq toliaferrode


text:
Occasional jitter when KinematicBody2D moved in _physics_process()

Godot version:
3.2.1 and 3.2.2

OS/device including version:
Windows 10, 8Gb RAM, Nvdia Geforce 920M (Driver version 425.31), GLES3 (probably occurs in GLES2 as well)

Issue description:
The docs say to use move_and_* functions in _physics_process(), but there is an occasional jitter that happens around every 2-3 seconds. Using move_and_* in _process() however doesn't have jitter.

Steps to reproduce:
Any kind of KinematicBody2D node when moved with move_and_* in the _physics_process(), but i have included a project that shows a comparison between:

move_and_slide() used only in _physics_process()
move_and_slide() used only in _process()
and ive also tried using Engine.get_physics_interpolation_fraction() to interpolate a KinematicBody2D that moves in _physics_process() (my code for this is based on https://www.kinematicsoup.com/news/2016/8/9/rrypp5tkubynjwxhxjzd42s3o034o8)

On my laptop what i see is that 1. jitters occasionally, 2. doesnt jitter at all, 3. jitters really rarely, much less noticeable than 1.
Minimal reproduction project:
Interpolation.zip

I also have Interpolation.tscn as an Autoload, which was my implementation for the interpolation fractions, I then found Engine.get_physics_interpolation_fraction() is already provided in godot for this very reason. My InterpolatedTransform2D.tscn node has a boolean that lets you switch between my implementation and the Engine's version of interpolation fraction (both of them are identical afaik).

Try installing, enabling and configuring lawnjelly's smoothing-addon and see if the issue persists.

See also https://github.com/godotengine/godot-proposals/issues/671.
Yes currently out of the box, with current Godot and physics, you will always get jitter from aliasing unless e.g. by chance your refresh rate matches your physics tick rate. This is well established and here are some possible solutions, all of which have pros and cons:

Fixed timestep interpolation
Semi fixed timestep (#30798)
Matching physics rate to refresh rate (reduz' preferred solution)

You can also get jitter from other sources, such as frame delta input / output delay variations, and physics behaviour.

@Calinou the plugin works really nice, but i have concerns about how to structure my scenes with that method.
Lets say i want to make a 2d platformer player scene. I may have either a KinematicBody2D or a RigidBody2D (or as you call them physics representation) as the root node of the player scene, with sprites (visual representation) as its children, this is the usual way i may structure it.
With the plugin i have to separate them as a *Body2D node and a Smoothing2D with the sprites as its children. This makes it confusing and cumbersome to move the player around in the editor view (needing to move both of them together). If i make a Node2D as a common parent to both the physics rep and visual rep then from an external script it becomes harder to add forces or add velocities to the *Body2Ds.
Do you have a suggested way of structuring nodes to solve this?
In my implementation, the node structures for the player would remain the same, but to add interpolation to it i just add the InterpolatedTransform2D node as a child to the player, and have two additional function calls in the players _physics_process().
Also another idea that preserves the old way of structuring would be to have:
Player
Smoothing2D
Sprite
where Smoothing2D has set_as_top_level(true), which makes it independent of the parents transforms.

@lawnjelly one of my monitors has one of those weird refresh rates of 59.97 something, and jitters on it. I cant set the physics rate to floating values in the editor, only integers.

Although I mentioned it, I'm not personally a fan of fixing physics ticks rate to the frame rate these days. It used to be used when e.g. consoles were using either NTSC or PAL, but with modern devices you can get all kinds of frame rates including variable. This is mainly due to the effects on gameplay.
If this was done properly in core, it would estimate in whole frame segments how physics ticks had passed on an update. So I don't think 59.97 would matter (there have always been those quirks).

Lets say i want to make a 2d platformer player scene. I may have either a KinematicBody2D or a RigidBody2D (or as you call them physics representation) as the root node of the player scene, with sprites (visual representation) as its children, this is the usual way i may structure it.
With the plugin i have to separate them as a *Body2D node and a Smoothing2D with the sprites as its children. This makes it confusing and cumbersome to move the player around in the editor view (needing to move both of them together). If i make a Node2D as a common parent to both the physics rep and visual rep then from an external script it becomes harder to add forces or add velocities to the *Body2Ds.
Do you have a suggested way of structuring nodes to solve this?
In my implementation, the node structures for the player would remain the same, but to add interpolation to it i just add the InterpolatedTransform2D node as a child to the player, and have two additional function calls in the players _physics_process().
Also another idea that preserves the old way of structuring would be to have:
Player
    |---Smoothing2D
            |---Sprite

where Smoothing2D has set_as_top_level(true), which makes it independent of the parents transforms.

I only recently changed the addon to artificially prevent this scene structure. Several people were using it already, and it can kinda work but can give nasty problems in some situations:
https://github.com/lawnjelly/smoothing-addon/issues/7
Perhaps I was overzealous on this, especially in 2D. But with 3D the maths rapidly becomes 'explosive' (numerically unstable would be better wording lol).
If you find a better way of doing it for your purposes, go for it, and give us feedback so we can improve it. You are absolutely right it is useful to be able to move things and see feedback in the editor. Perhaps that is possible to do with the current system with an editor notification, maybe we can get this working. :+1:

If you find a better way of doing it for your purposes, go for it, and give us feedback so we can improve it. You are absolutely right it is useful to be able to move things and see feedback in the editor. Perhaps that is possible to do with the current system with an editor notification, maybe we can get this working. 👍

@lawnjelly Have you tried my sample project? Its not as polished as yours but gives a cleaner node structure.
Interpolation.zip
Basically it acts as a component on any *Body2D but requires 2 additional function calls in the _physics_process().
It is based on www.kinematicsoup.com/news/2016/8/9/rrypp5tkubynjwxhxjzd42s3o034o8 almost entirely.

@lawnjelly Have you tried my sample project? Its not as polished as yours but gives a cleaner node structure.

Yup. The problem with that approach I think is that by setting the rigidbody / kinematic body in the interpolator you are also setting up a feedback loop with the physics. Drop your physics tick rate to 5 ticks per second and you'll see what I mean.
In godot there's no way to set the render transform without also setting the physics afaik, hence the  approach of decoupling the physics from the render object.
If instead you interpolate the icon directly, you might have more luck, but you will be dealing with a local transform relative to the parent, which is where the math can get kinda hairy in some situations.
You can on the face of it get a local transform from the previous transform to the current (of the physics rep in global space), and then interpolate this as the local transform of the icon, and I've used this kind of approach before. However there are some situations, especially in 3d where I had problems with it in terms of numerical stability, as far as I remember. This was what made integrating it into spatial such a pain.

authors: 2nafish117 Calinou lawnjelly


text:
CSGPolygon added at runtime will not render if set to Path mode.

Godot version:

3.3
OS/device including version:

Mint 19.3 Cinnamon OpenGL ES 3.0 Renderer: GeForce GTX 1060/PCIe/SSE2
Issue description:

I’m adding a CSGPolygon to a scene via script. It looks to render correctly if set to depth or spin. It will not render though if it’s set to path and set_path_node() is pointed to a path node that’s already in the scene tree. set_path_node() works as expected on a CSGPolygon already in the scene tree with no issues.
If the same scene is run in Godot 3.2.3 it'll throw the error _update_shape: Cannot get CSGBrush
Steps to reproduce:
Add a CSGPolygon to a scene via script, set to path, and set its path node.
Minimal reproduction project:

CSGPolygon not showing.zip

cc @madmiraal

This is not a bug. In the example provided, the path used in set_path_node() doesn't point to the Path object. The CSGPolygon object created in the script is added as a child of the Spatial object. Therefore, the Path needs to be set to "../../OldPath" not "../OldPath".

authors: Armadon7 Calinou madmiraal


text:
[Godot4] maybe should treat null as any variant type

Godot version
4.0.dev.20210924.official
System information
window10
Issue description
I try implement my MyVector class,
Steps to reproduce
If I code like this, all work well.

But if I want set default parameter and set data=null,
I don't know how to code it because syntax error:

Maybe I can not asign the function 'set_data' parameter 'value' type,
but I think it's not a good idea and also have a syntax

Minimal reproduction project
NullParamTest.zip
No response

Oh, I found can code like this:
func _init(size:=0,data=null):
	_init2(size,data)

func _init2(size:int, data):
	m_data.resize(size)
	match(typeof(data)):
		TYPE_INT,TYPE_FLOAT:
			for i in size:
				set_data(i,data)
		TYPE_ARRAY:
			assert(size<=data.size())
			for i in size:
				set_data(i,data[i])

There is a proposal about it: https://github.com/godotengine/godot-proposals/issues/162

func test(val:= null) # null type
func test(val = null) #any type

This looks like a valid issue, similar to #56217, and can reproduce on current master. The conversion to float is supposed to be attempted at runtime, so this should compile without error.

Yeah, after looking at it again, this is a bug.

authors: CsloudX KoBeWi cdemirer


text:
Profiler: the checkboxes seem difficult to tick

Tested on Godot 2.1 beta, Windows 10 64 bits
When testing the debugger, I found that ticking the checkboxes that toggle curves are not very responsive, I often have to click several times on them, even when the game is paused or profiling is stopped.
However, no problem when the game doesn't runs.

Not critical for the upcoming 2.1, so moving to the next milestone.

Is this still valid?

I think it's still valid, I found it difficult to tick profiler checkboxes for curves.
I think the problem increases the lower the checkbox is (for lines high up, it's easier, for the bottom line its argh why won't it tick!)

I'll close this as nobody else has been able to reproduce this recently. Please comment (with the issue template filled) if you can still reproduce this.

authors: Calinou Zireael07 Zylann akien-mga bojidar-bg


text:
iOS export throws the following error: "JSON text did not start with array or object"

Godot version
3.4.4
System information
macOs 12.2 Monterrey, M1
Issue description
Exporting to iOs doesn't work. Throws the following error:
2022-04-16 11:09:07.733 xcodebuild[12148:7643367] Error Domain=NSCocoaErrorDomain Code=3840 "JSON text did not start with array or object and option to allow fragments not set. around line 1, column 0." UserInfo={NSDebugDescription=JSON text did not start with array or object and option to allow fragments not set. around line 1, column 0., NSJSONSerializationErrorIndex=0}
xcodebuild: error: Unable to read project 'app.xcodeproj' from folder '/Users/xxx/projects/apks'.
    Reason: The project ‘appproject’ is damaged and cannot be opened due to a parse error. Examine the project file for invalid edits or unresolved source control conflicts.

Path: /Users/xxx/projects/apks/app.xcodeproj

xcodebuild (.ipa):
error: archive not found at path '/Users/xxx/projects/apks/app.xcarchive'
** EXPORT FAILED **

Tried creating a new project and I still get the same error.
Other exports work fine (android, html, macos, etc)
Steps to reproduce

Download latest export templates
Properly fill iOs options requirements (teamid, identifier, icons).
Hit Export Project.

Minimal reproduction project
No response

Have you found any workaround? I've tried to install previous version of engine 3.4.2, 3.3.4 and it does not fixed that problem. Maybe issue is a last Xcode version?

This was confirmed by cucaido on the Godot Forums: https://godotforums.org/discussion/29990/failed-to-export-ios-project-due-to-reason-the-project-x-is-damaged-and-cannot-be-opened

No way to export to ipa in 3.4.4 too unfortunately. Any hints on how to solve it manually?

As workaround, App Store Team ID should be not your email but ID like : 4CP59N327WZ

Actually free users do have an App Store Team ID, but it is not displayed in Xcode. For free users, Xcode displays your email whereas  or paid users, Xcode will display the App Store Team ID. But, free users still have a proper App Store Team ID. You need to create a Xcode project and open the .xcodeproj in a text editor and search for the key DEVELOPMENT_TEAM.
buildSettings = {
    ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
    ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor;
    CODE_SIGN_STYLE = Automatic;
    CURRENT_PROJECT_VERSION = 1;
    DEVELOPMENT_ASSET_PATHS = "\"Example/Preview Content\"";
****DEVELOPMENT_TEAM = 485FK5P5JK;
    ENABLE_PREVIEWS = YES;
...

You can view Team ID of the free "Apple Development" certificate in the macOS Keychain, open certificate and look for "organisational unit" value.

This issue seems to be mostly a UX issue, maybe we could make this more clear somehow. Until then, we've now documented this on the docs: https://github.com/godotengine/godot-docs/pull/7069

For others who arrive here--you likely need to populated your "TeamID" accessible from  https://developer.apple.com/account#membership

authors: Calinou JMedina90 Sc4pego4t bruvzg jreusser krupitskas lawrnce mhilbrunner vlyahovich


text:
Add Scots (lang code "sco") as a supported language in Godot

Operating system or device - Godot version:
Windows 10 - Godot 2.1 Stable
Issue description (what happened, and what was expected):
I tried to add my Scots translation of a game into my project, and it didn't recognize it, nor does it officially support.
Can this language be added to the list in 2.1.1. or whatever next version comes out?

@akien-mga You want help on the issue? (i.e. pretty bump)

authors: Cobradabest bojidar-bg


text:
Wildcards in function arguments

Given that we now have TK_WILDCARD in GDScript, and _ is officially banned as a variable ( @karroffel  ), I would like to request to be able to write this:
func handler(_, _):
  pass # Might actually complete this function one day

Instead of:
func handler(discard = null, discard2 = null):
  pass # Might actually complete this function one day

It's not merged yet btw.. 😄

@karroffel My bad -- anyway, I guess it would be merged given the community response on that other issue...

I still need reduz consent 😄

The PR #7583 has been merged by Akien a day ago. So this is a valid issue now.

I'm for pythonic
func handler(*args):
  pass

or even
func handler(*_):
    pass

in case of wildcard.

I prefer pythonic if we need to add this feature too
On Jan 24, 2017 13:33, "Yasha Borevich" notifications@github.com wrote:

I'm for pythonic
func handler(*args):
pass
or even
func handler(*_):
pass
in case of wildcard.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/7599#issuecomment-274857156,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF-Z21LKNune0boguhd0vjGFUjKh-BuCks5rVifSgaJpZM4LqHHh
.

I'm not speaking about varagrs, I'm speaking about using _ as a parameter name, to make an ignored value. i.e. for stuff like func do_stuff(_, second_param) over func do_stuff(something_not_going_to_be_used, second_param)

I think it's worth adding. It's making use of the new token and if (ever 😢) the lambda PR gets merged then this
var x = func(x,_): x

looks a lot sexier than
var x = func(x, y): x

because it's clear from the argument list that only the first param will be used.
I might take a look at how easy it is to implement that.

Note that you can avoid the warning with discarded arguments by leading the parameter name with an underscore (e.g. _arg_name instead of arg_name).

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.
The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.
If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

authors: Calinou ClikCode bojidar-bg jjay karroffel reduz


text:
MotionCollision get_angle() returns NAN

Godot version
master (e684d126ed6)
System information
Windows 10
Issue description
MotionCollision's get_angle() function can return NAN. According to the docs it should always return a positive value.
I believe I'm running into the same scenario as https://github.com/godotengine/godot/issues/8111 as applying a CLAMP within the acos() seems to fix the issue.
Steps to reproduce
In my case I had a platform that could rotate along the z axis and an independent character controller who's gravity would automatically adjust to the down vector of the rotating platform and the character controller would rotate towards the gravity so it's down vector also aligned with the platform's.
Using get_angle against the ground from the player's up direction would occasionally result in NAN at various platform rotations despite the vectors being perpendicular and normalized.
It would also seemingly result in weird behavior in move_and_slide() where the velocity would continuously have gravity added to it despite the character controller not moving.
In the minimal reproduction project there's a single scene with a platform that's named something along the lines of rotate me. If you run that project and slowly rotate the z axis from the remote tree eventually you should see an instance where "nan!" gets printed indicating it ran into the issue. It's very finicky and the character controller can fall through the platform so it might take a few tries.
Minimal reproduction project
GetAngle.zip

Ah, this has come up before, imo we should be using a safe_acos() function in the majority of cases in the engine (with input clamped). It's very easy for float error to get outside the -1 to 1 range and cause Nan, and clamping is pretty cheap outside bottleneck code.


text:
BIND_VIRTUAL_METHOD in GDextensions doesn't create a virtual function in GDscript

Godot version
4.1.1
System information
Pop!_OS 22.04
Issue description
Using BIND_VIRTUAL_METHOD script in _bind_methods() doesn't create a virtual function in GDScript.
Steps to reproduce

Create simple GDextension
Build it
Add to the project

//MyClass.h
#ifndef MYCLASS_H
#define MYCLASS_H

#include <godot_cpp/classes/node.hpp>

namespace godot{
class MyClass : public Node{
	GDCLASS(MyClass, Node)

protected:
	static void _bind_methods();

	void foo();

};
}

#endif

//MyClass.cpp
#include "my_class.h"

#include <godot_cpp/core/class_db.hpp>
#include <godot_cpp/variant/variant.hpp>

using namespace godot;

void MyClass::_bind_methods(){
	BIND_VIRTUAL_METHOD(MyClass, foo);
}

void MyClass::foo(){

}

After building the extension, you can MyClass in the editor. However, you can't override foo() function as GDscript doesn't see it and it isn't shown in documentation section.
Minimal reproduction project
N/A

Related issues:
godot/godot-cpp#910
godot/godot-cpp#1072
godot/godot-cpp#1199
godot/godot-cpp#910 mentions a possible work around by using call("_virtual_func") instead but even that isn't ideal when dealing with multiple related classes (and might not even work when overriding it in C++.)
This is especially annoying as documentation is also broken for GDExtension if I'm not mistaken and makes defining extendable classes quite difficult.
Reimplementing the GDVIRTUAL macros is impossible as Object::get_script_instance and the ScriptInstance class aren't exposed.

authors: SilicDev TrickyFatCat


text:
Shader causing crashes

Godot version
4.1.1
System information
Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Forward+) - integrated Intel(R) HD Graphics 620 () - Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz (4 Threads)
Issue description
Godot crashes whenever attempts to edit or run my shader are made. I sent the file to a friend on better hardware, he could replicate similar outcomes. Results vary from as many repetitions of the same 50 or so error lines as your ram can take (doesn't seem to be lines I wrote) to Godot simply crashing to the project failing to open properly.
Steps to reproduce
DoG_Shader.txt
Just run this as a shader within Godot (copy the text from this file into a blank shader) and try to run, edit, or just interact with it in almost any way. Using the debugger just fills the screen and ram with weird stuff, scrolling in the error log grinds the computer to a holt, needed to use task manager to get me out of that one.
Don't worry about what the parameters are, they don't effect the error. Sorry for messy code, I thought I should leave everything I had as it was when I first encountered the bug.
Minimal reproduction project
BuggyShader.zip

Thanks for the report. You can't use uniform parameters as inout function parameters. These lines:
colour_mask(mask.r, filter_colour.r, paper_colour.r, texture(SCREEN_TEXTURE, SCREEN_UV).r, mask);
colour_mask(mask.g, filter_colour.g, paper_colour.g, texture(SCREEN_TEXTURE, SCREEN_UV).g, mask);
colour_mask(mask.b, filter_colour.b, paper_colour.b, texture(SCREEN_TEXTURE, SCREEN_UV).b, mask);

all pass paper_colour (.r, .g or .b) as inout parameter, but it's an uniform and you can't modify them in the shader. The shader error spam is annoying and easily locks up the UI and it should probably have a separate issue made for it if it doesn't already have one.

Looks like the compiler will show the same error for every variant.
However the error shown is correct:

I think there are some ways to improve this:

Instead of error_print the whole shader, just print the relevant part (log whole shader to output is probably fine, but a large ui block the editor is not a good idea, maybe we should find some way to limit the error info's szie)
Find a way to stop compilation when one of many variants has error. (the worker doesn't have a return value so we should find another way to do this)
https://github.com/godotengine/godot/blob/0ca8542329888e8dccba89d59d3b728090c29991/servers/rendering/renderer_rd/shader_rd.cpp#L516-L517

2. Find a way to stop compilation when one of many variants has error.

I have an idea of preventing endless error spam, we can compile one variant first, and if that succeeded, we can continue using work pools.

authors: Mr-Pineapples bitsawer jsjtxietian


text:
Looping animation with collision:shape:radius/height causes Area2D not trigger

Operating system or device - Godot version:
Linux Mint 64x

Issue description:
If you play a looping animation from the AnimationPlayer

Area2D does not register the player when it enters.

According to a test, it appears when you set either the (CapsuleShape) collision shape radius or height as a looping animation (or if it just runs on a long animation) Area2D does not register the body.

If I try to add the Shape itself, there is no problems. Only the radius and height appears to make the body invisible to the Area2D.
Steps to reproduce:
Create a similar scene:

Create a script on Area2D and setup the signal.

Create an animation that loops and autoruns:

Notice that the Area2D does not register the RigidBody2D as it passes.

You are animating the collision shape? remember that the CS is a helper and the CollisionObject has its own shape.
If you want to animate the real shape, you may need to export some variable to animate and change the shape value with the setter (you can update the CollisionShape at the same time).
Still not sure if detection will work, smoothly at least.

Does it work if you use fixed-mode AnimationPlayer?
Also, note that the shape would be shared between all instances of the scene, since it is a Resource. Maybe it would start working if you duplicate it in _ready, or if you have only one instance?

@bojidar-bg using fixed happens too, and the effect manipulating the shape (using another variable) is the same, the new shape won't be used until you update the area position.
Adding a new track with the position, even without moving it, makes the area detect with the new shape size (like using set_pos).
Also happens using Tween.

Still affects Godot 3.0.2
The workaround in this case is to do position = position on the area when shape changes so the physics server becomes aware of the new shape.
I think shape changes should notify the owner, and the body set as "changed" to be checked again for collisions, this may solve many shape resizing bugs.

Thanks! I was struggling with this for almost two hours before I found this answer!
I'm still on Godot 2.1.4, and I was animating a ColisionShape2d inside an Area2d. It's a player projectile, a Laser that would cross the screen and stay active for around 2 seconds.
Its behavior was all over the place, as it killed the enemies half the time and half not. After I added set_pos(get_pos())  to the "fixed_process" of the laser, it seems to be working 100% of the time now.

@vandalk on Godot 2, CollisionShape nodes are just editor helpers and you should not use them in game but modify the shape resource instead (on 3 are more active nodes).
The position change should be done on the CollisionObject (Area2D in this case).

I can confirm this is still an issue in 3.0.6, but @eon-s's workaround seems to work reliably for me when using _physics_process;
func _physics_process():
	position = position

still seems to happen in 3.1.2

It was never cherrypicked there. It should be fixed in 3.2.x, however.

authors: Tybobobo bojidar-bg eon-s jackwilsdon securas vandalk


text:
Crash while adding fbx file to scene

Tested versions
reproducible in 4.4Dev-4
not reproducible in 4.4Dev-3
System information
Linux 6.10.11-2-MANJARO #1 SMP PREEMPT_DYNAMIC Thu Sep 26 03:28:26 UTC 2024 x86_64 GNU/Linux
Issue description
Importing seems to work but when I drag and drop fbx file to scene it crashes
it's same for glb and gltf files
do not know why but some other FBX files were working flawlessly could be because they were very small
Steps to reproduce
simply drag and drop fbx file to scene
Minimal reproduction project (MRP)
animation IK testing.zip

I don't have this problem on Windows 10
Godot v4.4.dev4 - Windows 10.0.19045 - Multi-window, 1 monitor - Vulkan (Forward+) - dedicated Radeon RX 560 Series (Advanced Micro Devices, Inc.; 31.0.14001.45012) - Intel(R) Core(TM) i5-4570 CPU @ 3.20GHz (4 threads)

It doesn't crash for me on v4.4.dev.gh-94321 [93112d811]

it doesn't crash on Mac M3Max 4.4 dev4

I just made a fresh install on 2 devices was hoping to have a system problem but it seems I have a hardware problem
device with crash:
Intel Core i7-4700HQ
NVidia Geforce GTX 850M
device with no crash:
AMD Dual-Core processor E450
AMD Radeon HD 6320

I don't get crash on 4.4-Dev6.
so I'm closing.

authors: JekSun97 MarkoGrbec fire snougo


text:
Navigating an OptionButton's PopupMenu using keyboard or controller causes flashing items

Tested versions
Godot_v4.3-stable, Godot_v4.4-dev5
System information
Godot v4.3.stable - Windows 10.0.19045 - Vulkan (Forward+ [This bug also appears using the compatibility renderer]) - dedicated NVIDIA GeForce RTX 4070 (NVIDIA; 32.0.15.6590) - AMD Ryzen 5 7600 6-Core Processor (12 Threads)
Issue description
Navigating an OptionButton's PopupMenu with a keyboard's arrow keys or a controller ocationally causes the items in the menu to quickly flash before settling on a seemingly random item. This glitchy behavior only happens durring the first input on that menu.
This issue is similar #42230
Steps to reproduce
Use a keybaord's arrow keys to navigate an OptionButton's PopupMenu.
This can be quickly done in Godot's project manager in the top right where it says: "Sort: Last Edited".
Reproducing this bug is inconsitant so it may take a few attempts.
Minimal reproduction project (MRP)
N/A

Sound to be same as this issue https://github.com/godotengine/godot/issues/94712#issuecomment-2282093214

Thanks for sending that link Giganzo, I belive this is the same issue as the one you mentioned.
I searched and searched for a long time but coudln't find anyone with the same issue.
This issue could probably be closed / merged with the one you mentioned unless anyone has anything else to add to this.


text:
Fix crash when restoring script editor state

Fixes the crash when restoring the script editor state.
closes #28307
closes #28288

Thanks!

source: gharchive/pull-request


text:
Fix non-looping animations not being reset properly in AnimationTreePlayer

When AnimationTreePlayer switches to new animation it never
seeks it to 0 which leads to problems with non-looping animations being
played just once.
This patch is direct approach fixing this problem.
It handles most common cases of occurance.
Closes #2199

I guess it would have been nice to name the parameter p_switched instead of switched, thus following the implicit convention used throughout the code. If you want to rebase to make this change it would be perfect, if not I'd say it's not a big issue either :)

I don't think that is worth it, as there is some lack of style throughout
all that code.
Feel free to do whatever change necessary, I will have no time digging into
this
much for some time.
On Fri, Dec 11, 2015 at 11:38 AM, Rémi Verschelde notifications@github.com
wrote:

I guess it would have been nice to name the parameter p_switched instead
of switched, thus following the implicit convention used throughout the
code. If you want to rebase to make this change it would be perfect, if not
I'd say it's not a big issue either :)
—
Reply to this email directly or view it on GitHub
https://github.com/godotengine/godot/pull/3036#issuecomment-163875272.

authors: akien-mga slapin


text:
Attempt to fix method calling in animation tree

This is my first pull request for open source so be gentle.
Fix for #25950
As i understand the system, caches applying to the same path are overriding each other since they are being held in a hash map, so duplicates are not working.
My solution is to make cache a hashmap of NodePath and set of TrackCache.

You linked wrong issue.

You linked wrong issue.

Fixed

So do think that issue #25950 is fixed
Amusingly i still can't call a method from animation in my project so i seem to have a different problem

Could you squash commits together, and edit the resulting commit log to be more descriptive?
See http://docs.godotengine.org/en/latest/community/contributing/pr_workflow.html and https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-logs-with-readability-in-mind

I don't think turning this cache into a set is the correct solution. The actual cause for the bug seems to be the one described at https://github.com/godotengine/godot/issues/25950#issuecomment-473758324
Since different function tracks share the same (empty) name, they override each other. So I think the proper fix would be to make sure that each track has a unique name.

Not sure why, but when patched on top of f4e370189 this PR breaks my animation tree. All characters are TPosed. Animation player on importing a GLB works fine but when I add an animation tree, things start breaking.

@IllusiveS Is this still desired? If so, it needs to be rebased on the latest master branch, and the feedback should be addressed.
If not, abandoned pull requests will be closed in the future as announced here.

@aaronfranke #25950 was closed, does this PR address a separate issue?

Oh, I see. Closing as fixed by a mysterious unknown commit :eyes:

authors: IllusiveS KoBeWi aaronfranke aclave1 akien-mga


text:
don't install libevdev-dev for travis build

Don't need that anymore, as the dependancy on it has been removed.

Indeed.

authors: Hinsbart akien-mga


text:
[macOS] Add "Khronos Validation Layer" config file to the tools .app bundle template.

Adds "Khronos Validation Layer" config file required to load validation layer if its library is place inside the editor .app bundle, to distribute it without need for the LunarG SDK installation.

I don't think we should/would distribute Godot with Vulkan validation layers though. Validation layers are for the engine development itself, engine users shouldn't need them (and release builds should likely be made not to attempt loading them at all for performance).

authors: akien-mga bruvzg


text:
Fix C# List marshalling

Fixes #42926
Fixes #43132
Fixes #41309
Marshalling List<T> was not working when T was a ValueType (like int, bool or enums).

Thanks!

authors: akien-mga raulsntos


text:
Correct docs for Environment background keep mode

The docs incorrectly stated that KEEP was the fastest mode. This is not the case with modern hardware.
Mentioned in #23677.
See e.g.
https://stackoverflow.com/questions/37335281/is-glcleargl-color-buffer-bit-preferred-before-a-whole-frame-buffer-overwritte
https://stackoverflow.com/questions/2538662/how-does-glclear-improve-performance

The docs incorrectly stated that KEEP was the fastest mode. This is not the case with modern hardware.

When I tested it on desktop, Keep was the fastest mode in both 2D and 3D. This might not apply to mobile though (due to the use of TBDR).

When I tested it on desktop (GTX 1080), Keep was the fastest mode in both 2D and 3D. This might not apply to mobile though (due to the use of tile-based rendering).

It's possible we aren't doing the glClear properly. Afaik it can be important to clear all the bits (at once), stencil etc just in case. If anything is in the buffer and doesn't get cleared, it may end up having to preserve it.

Thanks!

Cherry-picked for 3.4.4.

authors: Calinou akien-mga lawnjelly


text:
Prevent error emitting when clicking on empty space on EditorFileDialog

Prevents error when user clicking on empty space in that dialog, e.g.:

Duplicate of #60906 (in part).

authors: Chaosus akien-mga


text:
Fixed 2d movement of gizmos after node was rotated

Fixed problem with moving gizmos after it was rotated. Described here: #62732

This doesn't seem to work well with some angles:

This happens because the node will follow cursor exactly on one axis and then moves by rotated value on another axis, which might be multiple times longer, depending on angle.
Although not sure how it could be solved 🤔

This is not a proper fix. It may seem to fix some trivial cases (with the default scale/skew) but it won't fix general case. Also the drag is still considered in the global space which makes even some trivial cases fail, for example try a single Node2D as a scene root with rotation set to 90 degrees and try to use any these movement gizmos.
I guess a proper way of solving this would be to make different global/local modes. And in the local mode axes of course can end up being not "globally perpendicular" (depending on the global transform of the given Node2D).
Showing always-perpendicular-axes rotated according to the global rotation of the x axis of the node (like it's currently done) suggests these are local axes but that's not true in general case (again: axes can end up being to perpendicular e.g. just after skewing).

Thanks for feedback! I will dig deeper and try to come up with more robust solution

authors: KoBeWi ZakharE kleonc


text:
Uses StringName in GDExtension perf critical instance creation & method/properties setter/getter

Godot-CPP twin PR: https://github.com/godotengine/godot-cpp/pull/896
Function such as gdnative_classdb_construct_object would obsviously benefit from using StringName instead of char * (it's the whole point of StringName after all !)
Functions such as gdnative_variant_get_ptr_builtin_method also benefit in case it is called as part of the actual function call (e.g. in dynamic language or if the output of gdnative_variant_get_ptr_builtin_method is not cached)
On the other hand, I've kept use of char * in classdb_register_extension_*, GDNativeExtensionClassGetVirtual functions and GDNativePropertyInfo/GDNativeMethodInfo/GDNativeExtensionClassMethodInfo structures.
The reason is those are not performance critical:

classdb_register_extension_* are called once when the extension is initialized
GDNativeExtensionClassMethodInfo is only used by classdb_register_extension_*
GDNativeExtensionClassGetVirtual is cached by classDB
GDNativePropertyInfo/GDNativeMethodInfo are only used by ClassDB::get_method_list/ClassDB::get_property_list which are called by stuff like editor a gdscript analyzer, so I guess this is not used at release. However we may want to convert it anyway to speed up gdscript analyzer (though I don't know this code at all so it's very possible this would bring no real performance).

I can update the PR if you think it would be better for consistency to use StringName everywhere.

I've said this in the past. I think all these methods should receive StringName. IIRC, the char * conversion (StringName(const char *)) copy ctor) doesn't support Unicode. Of course, the conversion could be fixed, but it's easier to just receive the StringName.

@neikeq good point, I'm convertion everything to StringName then ;-)

@neikeq I've updated the PR, now what we have:
GDExtension C API uses StringName instead of const char * (some places uses String depending on what is used by ClassDB, e.g. hint_string)
GDNativeExtensionClassMethodInfo has been simplified by removing the get_argument_type_func/get_argument_info_func/get_argument_metadata_func.
The main reason for that is simplifies the memory management of the String/StringName objects: GDNativeMethodInfo/GDNativePropertyInfo/GDNativeExtensionClassMethodInfo are now only used as argument to to the lassdb_register_extension_* method or for the get_property_list_func/free_property_list_func.

for lassdb_register_extension_* the GDExtension user know he can free the resources once the function has return
for get_property_list_func/free_property_list_func, the free_property_list_func is the obvious place to do the resource cleanup

On top of that, this simplifies the implemention for GDExtension user (it's much simpler to pass an array of GDNativePropertyInfo that to pass a function pointer that should construct a GDNativePropertyInfo !).

@bruvzg can I merge it then ?

It looks like this affects https://github.com/godotengine/godot/issues/61968, right?

@Bromeon exactly ! I didn't even know about this issue :smile:

Maybe also will help with https://github.com/godotengine/godot/issues/68375

I feel this PR was kind of pointless. None of these functions are performance critical and since StringName construction still needs to hash the cstring, it would have allowed you to optimize this on the side of the language bind. Now this is probably a lot more complex to implement.

Even worse, if you want to keep a hashtable on the extension side to optimize, how are you going to use the StringName for this?

Maybe one possibility could be to have a C function to get some uint64_t from the stringname you can use for hashing.

... and since StringName construction still needs to hash the cstring, it would have allowed you to optimize this on the side of the language bind.

Even worse, if you want to keep a hashtable on the extension side to optimize, how are you going to use the StringName for this?

I don't understand, can you explain these optimizations in more details? Some pseudo-code may help.

@neikeq StringName contains a pointer to a string table, which is what makes it fast. If you use it from GDExtension directly its probably going to be slower because you have to call the function pointers to compare it.
I wonder maybe each implementation can implement comparison operators just checking the memory (pointer). If this is the case it should be fine.
Either case, if there is consensus on using StringName for these things from those implementing extensions, I won't oppose. I just feel it may not have been necessary.

I wonder maybe each implementation can implement comparison operators just checking the memory (pointer). If this is the case it should be fine.

This is what I do from C#. StringName has a very simple layout, just a pointer.

@neikeq I have no idea how easy is to expose this to different languages, but if extension implementers are fine doing this, then I don't see the problem with this PR.

authors: Bromeon Zireael07 neikeq reduz touilleMan


text:
Fix the order in which additional transformations are applied

...in Matrix3 and Transform.
This is a part of the breaking changes proposed in PR #6865, solving the issue regarding the order of affine transformations described in #2565. This PR also fixed the affected code within Godot codebase. Includes improvement to documentation too.

Example project demonstrating the issue:
godot_test.zip


text:
[3.x] Add 3D occlusion queries

This PR makes it possible to use occlusion queries to cull away parts of the scene that are not visible. While not being quite as high quality as the manual occluders we already have this has the advantage that it can be enabled by clicking a simple check box in the settings.
The visibility of culled objects is checked with their bounding box every frame, if the bounding box would be visible the renderer checks the visibility of the original asset in the next frame. Especially in scenes with badly optimized 3d assets that have too many vertices, this can result in nice performance gains (see screenshot below).
Note that the performance of occlusion query can vary a LOT on different hardware & drivers, so I included a settings option to disable this feature depending on the GPU vendor. I also hard-limited this feature to meshes that have more than 256 vertices, due to the overhead of OQ.
The current implementation is as general-purpose as I could make it, with OQ enabled for pretty much all meshes. I have two branches that are more complicated and try to be smarter about this (e.g. exclude animated meshes, etc.) but after a LOT of ~professional testing~ trial and error, I think that this version is better for most use cases.
Something that I will probably need to change is the way I generate the query objects, I left a comment at the relevant line of code. Would greatly appreciate feedback on that in particular.

Screenshot
Sorry for the lackluster image, This is the best I could do on short notice. If I remember I will replace this with a better screenshot tomorrow when I take some time to work through my old PRs.
Anyways, this shows the GPU load in a scene that has a bunch of unoptimized meshes. The valleys are when I had a wall between me and the hero assets, the peaks are when I could see them.
Note that this image was taken with the depth pre-pass disabled. If I were to enable it the difference would be even bigger, since the pre-pass needs to evaluate all the vertices a second time.

This PR was sponsored by Ramatak with 💚

Please amend the commit message to be more explicit (the detailed PR description is great, but commit messages also matter - see CONTRIBUTING.md).

Should be fun to try! :smiley:
Good overview of occlusion queries (especially popping and sync issues):
https://www.youtube.com/watch?v=wTTe78DBb1U

Anyways, this shows the GPU load in a scene that has a bunch of unoptimized meshes

Note that Windows' GPU usage readout is wildly known to be inaccurate. I suggest using external tools such as RTSS instead.

Yeah, I know. I only used the task manager to get a quick screenshot for the PR description. During development I used a number of different tools to measure performance, but when I wrote the PR description it was quite late and I was too tired to set up everything and then take good screenshots.
The taskmanager screenshot was quick, is close enough to the results I got with renderdoc, and should be easy to understand even for people who are not overly familiar with profiling (HP wanted something he could show to people).
Thanks for the docs feedback, I will change that and the commit message once I am back at my PC.

authors: Ansraer Calinou akien-mga lawnjelly


text:
CI: Install Android NDK r23c explicitly, with caching

Related to https://github.com/godotengine/godot-cpp/pull/1314, where we don't rely on scons to download the NDK.
I wanted to enable the caching feature from https://github.com/nttld/setup-ndk, but it doesn't seem to work together with link-sdk. So the gain from this change is just some clarity, it may not speed things up.
Edit: Keeping as draft for now, blocked by https://github.com/nttld/setup-ndk/issues/518. If we can get the cache to work, then this becomes useful.

Keeping as draft for now, blocked by https://github.com/nttld/setup-ndk/issues/518. If we can get the cache to work, then this becomes useful.

The maintainer on setup-ndk seems busy and I don't want to add undue pressure. There aren't too much benefits from this PR as is so shelving it for now.


text:
Windows: Fix mouse handling regression

Fixes a regression from #84363. Admittedly, when I made such PR, I was unaware that there were two different flags for controlling windows' activation/focus. It turned out that the flag that I neglected was precisely the one involved when dealing with capture mouse mode. The concern is that by both being unified now, there's potential for other regressions in the other features that depended on the split (either before or after this PR), or maybe the original PR plus this one just makes it all solid.
Fixes #90359.

Please have a look at #90334 which goes in the opposite direction. Which of the two PRs would be preferable?

Please have a look at #90334 which goes in the opposite direction. Which of the two PRs would be preferable?

It seems to me like this PR (90382) implements what you suggested in https://github.com/godotengine/godot/pull/90334#pullrequestreview-1985129018, which also seems like the best solution to me.
I don't see the point of having 2 members for this if they need to be in sync.

Thanks!

authors: RandomShaper Sauermann akien-mga


text:
Add notes in Object documentation about TranslationServer methods

Almost every time my static functions are not compliant, it's because of tr(). Added a documentation note to direct people to TranslationServer for methods that just work.

Thanks!

authors: MewPurPur akien-mga


text:
fixing linters and test in cassandra datasources

Description:

prequisite to closing #964
this PR has been made to separate concerns of different datasources in different PRs.

Checklist:

[x] I have formatted my code using  goimport  and  golangci-lint.
[x] All new code is covered by unit tests.
[x] This PR does not decrease the overall code coverage.
[x] I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

It's a limited version of #1252, where PR and discussions already happened

#1252

it was split into multiple PR because of https://github.com/gofr-dev/gofr/pull/1252#issuecomment-2520265141

authors: ccoVeille coolwednesday

repo: gofr-dev/gofr


text:
sending an Accept header when using the REST API fails with a 406

It seems the REST API fails if an Accept header is passed.
e.g. this works fine:
curl -vk -u myuser:mypwd http://localhost:3000/api/v1/user/repos

however this fails with a 406
curl -vk -H "Accept: application/json" -u myuser:mypwd http://localhost:3000/api/v1/user/repos
...
< HTTP/1.1 406 Not Acceptable

it'd be nice to be able to pass an Accept header along too (so its easier to invoke the REST API via a browser in AngularJS ;)

Thanks for the report! I'm not quite sure what's going on with "Accept: application/json", but will take a look.

Many thanks! I've been trying to figure out how to use $http in angularjs and not send any Accept header at all and so far no luck :)

Hmm... it's strange, based on current master version 0.6.1.0714,

curl -vk -H "Accept: application/json" -u myuser:mypwd http://localhost:3000/api/v1/user/repos

returns correct JSON results.

Close for now, but open for continuing discussion.

authors: Unknwon jstrachan

repo: gogits/gogs


text:
webhook https_proxy http_proxy

hello,
it seem that the webhook post doesn't use the environements variable HTTPS _PROXY and his friends ...
setting a slack integration only work it host has direct access to the net.
Gogs Version: 0.9.71.0809, env debian.

Hi, can you give more context? I don't understand your purpose here.

hello,
I have setup a slack integration, to have the webhook posting the push to slack. It work only if the gogs host have a direct access to the internet. If it is behind a firewall that block outgoing http request, we have to use a proxy. On linux this is set via environment variables.
that's
HTTP_PROXY="http://myproxy:8080
HTTPS_PROXY="https://myproxy:8080
NO_PROXY="localhost"
if that environment variables exists the host should use the proxy to make the http request to slack.
and that don't work with gogs, it make direct request that are blocked by the firewall.
gogs is realy cool, easy to setup with nginx, fast and efficient . good job .

Thanks for the explanation, any references how to use these environment variables?

in ruby the http library use it automagicaly! :
Net::HTTP will automatically create a proxy from the http_proxy environment variable if it is present. To disable use of http_proxy, pass nil for the proxy address.
and it seems that go do it also, look at
http://stackoverflow.com/questions/14661511/setting-up-proxy-for-http-client

I too would like to see this feature added - although I think the proxy settings should be added to the configuration file, maybe specifically under the webhook section.
For example:
[webhook]
HTTP_PROXY=http://myproxy.com:8080
HTTPS_PROXY=http://myproxy.com:8080
NO_PROXY=localhost,mylocaldomain.com

Then, Webhooks would check their domains against NO_PROXY, and if they don't match, would then use HTTP_PROXY for http webhooks, and HTTPS_PROXY for https webhooks.
In this way, Gogs running on a Private or Firewalled server, could still call external Webhooks such as Slack.

@Dortamur sounds interesting! But what's advantage of setting proxies in global config than individual webhook?

authors: Dortamur Unknwon tophe


text:
更新报错问题

原本的版本是v0.5.5_linux
更新程序最新v0.5.11_linux , 出现报错, 还原为V0.5.5_linux则无此报错
(更新v0.5.9_linux也出现同样现象)
2015/01/27 10:15:40 [T] Custom path: /opt/gogs/custom
2015/01/27 10:15:40 [T] Log path: /opt/gogs/log
2015/01/27 10:15:40 [I] Gogs: Go Git Service 0.5.9.1213 Beta
2015/01/27 10:15:40 [I] Log Mode: Console(Trace)
2015/01/27 10:15:40 [I] Redis Enabled
2015/01/27 10:15:40 [I] Memcache Enabled
2015/01/27 10:15:40 [I] Cache Service Enabled
2015/01/27 10:15:40 [I] Session Service Enabled
2015/01/27 10:15:40 [I] SQLite3 Enabled
2015/01/27 10:15:40 [I] Run Mode: Production
panic: template: user/dashboard:21: function "ActionDesc" not defined

goroutine 1 [running]:
html/template.Must(0x0, 0x7f623ce52580, 0xc2084a3a80, 0x0)
        /usr/local/go/src/html/template/template.go:304 +0x50
github.com/Unknwon/macaron.compile(0xc2081882c0, 0x13, 0x0, 0x0, 0xc2081882e0, 0x2, 0x2, 0xc2081ca010, 0x1, 0x1, ...)
        /home/wuwen/Applications/go/src/github.com/Unknwon/macaron/render.go:245 +0x400
github.com/Unknwon/macaron.(*templateSet).Set(0xc208084420, 0xccf900, 0x7, 0xc20810e1c0, 0x0)
        /home/wuwen/Applications/go/src/github.com/Unknwon/macaron/render.go:270 +0x51
github.com/Unknwon/macaron.renderHandler(0xc2081882c0, 0x13, 0x0, 0x0, 0xc2081882e0, 0x2, 0x2, 0xc2081ca010, 0x1, 0x1, ...)
        /home/wuwen/Applications/go/src/github.com/Unknwon/macaron/render.go:340 +0x248
github.com/Unknwon/macaron.Renderer(0xc20847d440, 0x1, 0x1, 0x0, 0x0)
        /home/wuwen/Applications/go/src/github.com/Unknwon/macaron/render.go:377 +0xea
github.com/gogits/gogs/cmd.newMacaron(0x55f990)
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/cmd/web.go:112 +0x6a2
github.com/gogits/gogs/cmd.runWeb(0xc208086180)
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/cmd/web.go:162 +0x34
github.com/codegangsta/cli.Command.Run(0xcf93e0, 0x3, 0x0, 0x0, 0xd55e90, 0x15, 0xe25d90, 0x64, 0x0, 0x0, ...)
        /home/wuwen/Applications/go/src/github.com/codegangsta/cli/command.go:101 +0xd6a
github.com/codegangsta/cli.(*App).Run(0xc20801f860, 0xc20800a000, 0x2, 0x2, 0x0, 0x0)
        /home/wuwen/Applications/go/src/github.com/codegangsta/cli/app.go:154 +0xbab
main.main()
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/gogs.go:41 +0x397

goroutine 5 [select]:
github.com/gogits/gogs/modules/log.(*Logger).StartLogger(0xc208070000)
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/log/log.go:231 +0x369
created by github.com/gogits/gogs/modules/log.newLogger
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/log/log.go:161 +0x13a

goroutine 7 [chan receive]:
github.com/gogits/gogs/modules/mailer.processMailQueue()
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/mailer/mailer.go:53                                               +0x69
created by github.com/gogits/gogs/modules/mailer.NewMailerContext
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/mailer/mailer.go:47                                               +0xd1

goroutine 8 [select]:
github.com/gogits/gogs/modules/log.(*Logger).StartLogger(0xc208135040)
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/log/log.go:231 +0x369
created by github.com/gogits/gogs/modules/log.newLogger
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/log/log.go:161 +0x13a

goroutine 183 [chan send]:
text/template/parse.lexSpace(0xc2083dea80, 0xe85370)
        /usr/local/go/src/text/template/parse/lex.go:339 +0xdd
text/template/parse.(*lexer).run(0xc2083dea80)
        /usr/local/go/src/text/template/parse/lex.go:198 +0x5d
created by text/template/parse.lex
        /usr/local/go/src/text/template/parse/lex.go:191 +0x1ac

goroutine 12 [select]:
github.com/gogits/gogs/modules/cron.(*Cron).run(0xc20807cac0)
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/cron/cron.go:162 +0xa65
created by github.com/gogits/gogs/modules/cron.(*Cron).Start
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/cron/cron.go:137 +0x3d

goroutine 19 [chan receive]:
database/sql.(*DB).connectionOpener(0xc208076460)
        /usr/local/go/src/database/sql/sql.go:589 +0x4c
created by database/sql.Open
        /usr/local/go/src/database/sql/sql.go:452 +0x31c

goroutine 49 [syscall, locked to thread]:
runtime.goexit()
        /usr/local/go/src/runtime/asm_amd64.s:2232 +0x1

goroutine 13 [select]:
github.com/gogits/gogs/modules/log.(*Logger).StartLogger(0xc20807df80)
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/log/log.go:231 +0x369
created by github.com/gogits/gogs/modules/log.newLogger
        /home/wuwen/Applications/go/src/github.com/gogits/gogs/modules/log/log.go:161 +0x13a

You should delete old template files first, then move the new ones to old position.

reopen if you want to continue discussing.

authors: Arylo Unknwon


text:
I18n not possible, config files for translations not loaded correctly

when adding i18n functionality for my website, I immediately got these errors
with my docker image frokm klakegg as well as the version locally installed
sven@kali:~/development/hugo$ hugo --verboseLog
Error: add site dependencies: load resources: loading translations: "/home/sven/development/hugo/i18n/de.yaml:1:1": failed to load translations: unsupported file format
Total in 2 ms
sven@kali:~/development/hugo$ make up
docker run  -it -v "/home/sven/development/hugo":/src -p 1313:1313 "klakegg/hugo:0.111.3-ext-asciidoctor" server  --disableFastRender --renderToDisk --verbose
Error: add site dependencies: load resources: loading translations: "/src/i18n/de.yaml:1:1": failed to load translations: unsupported file format int
make: *** [Makefile:11: up] Fehler 255
sven@kali:~/development/hugo$ hugo --debug
Error: add site dependencies: load resources: loading translations: "/home/sven/development/hugo/i18n/de.yaml:1:1": failed to load translations: unsupported file format
Total in 3 ms
sven@kali:~/development/hugo$ hugo --verbose
Error: add site dependencies: load resources: loading translations: "/home/sven/development/hugo/i18n/de.yaml:1:1": failed to load translations: unsupported file format int
Total in 2 ms

What version of Hugo are you using (hugo version)?

$ hugo version
hugo version
hugo v0.111.3+extended linux/amd64 BuildDate=2023-03-16T08:41:31Z VendorInfo=debian:0.111.3-1

### Does this issue reproduce with the latest release?
no tested yet

And this is what the file looks like:

params:
baseURL: "http://wehrend.uber.space/"
languageCode: "en-us"
title: "Bits & pieces - Sven Wehrend"
theme: "hugo-book"

menu:
  before:
  - name: "Web-Blog"
    url: "/posts/web/"
    weight: 10

menu:
  before:
  - name: "Synth-Blog"
    url: "/tags/synth/"
    weight: 20

Please use the forum (https://discourse.gohugo.io/) for questions and troubleshooting. When you create a topic on the forum, make sure you provide a link to your project repository.

Dear Joe,
does that mean it is fixed in the current version?
I am limited to those docker releases from klagegg, and I think 0.111.3 was
the last version there.
Should I update it...?
Best,
Sven Wehrend
On Wed, Sep 4, 2024, 16:25 Joe Mooring @.***> wrote:

Please use the forum (https://discourse.gohugo.io/) for questions and
troubleshooting. When you create a topic on the forum, make sure you
provide a link to your project repository.
—
Reply to this email directly, view it on GitHub
https://github.com/gohugoio/hugo/issues/12826#issuecomment-2329215544,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAMEREGQTE2OFHSI2UHS3WDZU4J7FAVCNFSM6AAAAABNUMB4SSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRZGIYTKNJUGQ
.
You are receiving this because you authored the thread.Message ID:
@.***>

does that mean

What it means is, please use the forum (https://discourse.gohugo.io/) for questions and troubleshooting.

authors: jmooring wehrend

repo: gohugoio/hugo


text:
Add functions to partition (page) slices

I propose to add some new functionality to split a slice of pages into chunks of a given fixed length plus possibly a shorter remainder.
Use cases include listings and multi-column row-based tabular layouts.
As far as I know there is right now no such functionality exposed to the templates API. The new functions should be straight-forward to use compared to a custom implementation by lower-level template primitives. It is expected to be faster and less error-prone too and, as a built-in readily available to all Hugo users.
This topic came up first with the bug report #11553 and it solves the issue raised there.
That bug is related to #11131 which broke a custom template-based implementation.
An implementation can be found at PR #12661 (resp. the branch https://github.com/raoulb/hugo/tree/partition).
With this post I'd like to start the discussion on what needs to be changed to get this functionality merged.

Please provide an example of how you would use the new template function in a template (e.g., /layouts/_default/list.html).

Thanks for this; this needs to put on hold for a little. I need to think a little about the "future" of the collections package. This package was implemented before Go generics, and I don't want to add more complexity/generics code if we don't need to. For funcs that operate on slices only, e.g. Partition, I don't see why we cannot just delegate to something ala:
https://pkg.go.dev/github.com/bep/helpers@v0.5.0/slicehelpers

Please provide an example of how you would use the new template function in a template (e.g., /layouts/_default/list.html).

I made a minimal example which can be found here
An envisioned use case might look like:
  {{ range $.Site.RegularPages | partition 3 | first 2 }}

Some heading

      {{ partial "fancy.html" . }}

  {{ end }}

where the fancy.html partial might implement a complex layout like a slideshow or similar.
The minimal example here will look like:

Thanks for this; this needs to put on hold for a little. I need to think a little about the "future" of the collections package. This package was implemented before Go generics, and I don't want to add more complexity/generics code if we don't need to. For funcs that operate on slices only, e.g. Partition, I don't see why we cannot just delegate to something ala:
https://pkg.go.dev/github.com/bep/helpers@v0.5.0/slicehelpers

I'd be fine with skipping the collections part (i.e. partition) and for now just add the PartitionWith functions on Pages.

For funcs that operate on slices only, e.g. Partition, I don't see why we cannot just delegate to something ala:
https://pkg.go.dev/github.com/bep/helpers@v0.5.0/slicehelpers

I guess we can? I'd try to implement this once we decide to use that dependency.

Is there any progress on this?

authors: bep jmooring raoulb


text:
Use chroma 0.3.0

Chroma has a new minor release as of yesterday https://github.com/alecthomas/chroma/releases
I am opening this issue because one of the bugs I reported there got fixed, so I’d love to see this in Hugo soon.

Thanks!


text:
Applications API v2

Context:
Until now, the information about enabled CaraML applications in the MLP deployment was persisted in the DB and retrieved from there via an API call. In the hindsight, that wasn't the best design decision, because it was required to add DB migration script for adding/modifying the app configuration, which in turn required releasing the new version of the mlp app.
Also, the information about the available CaraML applications conceptually belongs to the deployment configuration, because the same version of CaraML can be deployed with a different subset of CaraML applications (e.g. it should be possible to deploy CaraML with Merlin, but without Turing etc.). It's harder to achieve this, because the DB migrations are strictly versioned.
This MR attempts to solve this issue by introducing Applications API v2, which retrieves the information about CaraML applications from the runtime configuration and doesn't persist this data in the database. This refactoring is also done in preparation to embed CaraML microfrontends into the mlp host UI.
Changes:

Refactored CaraML API layer to support both v1 and v2 endpoints simultaneously
Refactored MLP UI and @gojek/mlp-ui components library to use /v2/applications instead of /v1/applications
Refactored MLP landing page to only show information about the resources if the application that owns such resources (i.e. models -> Merlin, routers -> Turing etc) is enabled

Next Steps:
Since the frontends of CaraML apps are still using /v1/applications, it's required to keep this API endpoint until these apps are updated to use the new version of @gojek/mlp-ui. After it's done, Applications API v1 can be completely removed from the codebase.

@deadlycoconuts thank you for the review 🙌

repo: gojek/mlp


text:
Reflect current state of project in the README.

People are getting confused on what happened with Bash version after we rewrote it in Rust #191 (I got emails) and which version should be used at the moment.
We should add a section to the README that'll reflect current state of the 3.x development, suggest to keep using 2.x until we release 3.x and how to try 3.x builds.

Damn, we should have used a 3.x branch :-(

repo: gojuno/mainframer

license_source: bigquery


text:
Add test suite.

Closes #8.
PR adds basic tests and "test runner" to the project. Design is pretty straightforward, I've added support for sshing any machine and for now it's configured to run against localhost as a "remote build machine", but if we'll find out that neither Travis nor CircleCI allow us run sshd, we'll setup separate machine and then we'll be able to encrypt ssh key for connection from Travis or CircleCI using their apis for encryption.
I don't have permissions to add CI like Travis or CircleCI fluck-fluck, so we'll figure out that later in a separate PR.
For now we can run test suite locally (for each PR): bash test.sh, this is how it looks like:

cc @dmitry-novikov PTAL (not blocking reviewer).

There is a medication for what you have, my friend.


text:
runtime: gcAssistAlloc can call itself recursively

This occurs in Go 1.5.1 possibly 1.5.0 as well.
gcAssistAlloc may decide to sleep for 200us. This results in allocating a timer object and eventually ending up in gcAssistAlloc again.
gcAssistAlloc -> time.Sleep -> builtin.new -> mallocgc -> gcAssistAlloc -> ...
sanitized snippet from debug/pprof/heap?debug=1
#       0x123456        time.Sleep+0x37                 go/gc/src/runtime/time.go:53
#       0x123456        runtime.gcAssistAlloc+0x23e     go/gc/src/runtime/mgcmark.go:295
#       0x123456        runtime.mallocgc+0x535          go/gc/src/runtime/malloc.go:711
#       0x123456        runtime.newobject+0x42          go/gc/src/runtime/malloc.go:760
#       0x123456        time.Sleep+0x37        go/gc/src/runtime/time.go:53
#       0x123456        runtime.gcAssistAlloc+0x23e     go/gc/src/runtime/mgcmark.go:295
#       0x123456        runtime.mallocgc+0x535       go/gc/src/runtime/malloc.go:711
#       0x123456        runtime.newobject+0x42      go/gc/src/runtime/malloc.go:760

Is this fixed now?  In both 1.5 branch and tip?

Yes. It was fixed on master by 65aa2d6 and on the 1.5 branch by c257dfb.

The fix on the 1.5 branch (c257dfb) isn't quite correct. We were able to verify that it reduces but does not eliminate the issue. I believe the recursion can still happen at the beginning of a GC cycle when gp.gcscanwork is also small or zero. I'm working on a better fix, which we'll try to verify more thoroughly.

Fixed (for real this time) on release branch by 71a7647.

authors: aclements deft-code ianlancetaylor

repo: golang/go


text:
x/mobile: IPHONEOS_DEPLOYMENT_TARGET is set to 8.3

can we set the deployment target to a lower version?
or it is required to be 8.3?

@rakyll, is there a particular reason that the deployment target is set to 8.3?

Our builds have never been tested on devices prior to 8.3.

authors: hyangah pftbest rakyll


text:
syscall, x/sys/unix: Time(), Utime(), Getrlimit() use invalid syscall number on linux/arm

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?
any

What operating system and processor architecture are you using (go env)?
linux/arm

What did you do?
(Use play.golang.org to provide a runnable example, if possible.)
nothing

What did you expect to see?
invalid syscall numbers are not used

What did you see instead?
invalid syscall numbers are used

As far as is known, linux/arm sycalls are slightly limited.
Here is the list taken from kernel header:
/*
 * The following syscalls are obsolete and no longer available for EABI.
 */
#if !defined(__KERNEL__)
#if defined(__ARM_EABI__)
#undef __NR_time
#undef __NR_umount
#undef __NR_stime
#undef __NR_alarm
#undef __NR_utime
#undef __NR_getrlimit
#undef __NR_select
#undef __NR_readdir
#undef __NR_mmap
#undef __NR_socketcall
#undef __NR_syscall
#undef __NR_ipc
#endif
#endif

I confirmed time, utime and getrlimit are used, others are not used in both syscall package and x/sys/unix package.
I also suggest such syscall numbers should be removed from these packages on linux/arm.
(except syscall package? because of compatibility promise)
Thank you.

Please show a complete program which misbehaves, and the strace -f ./yourbinary output when run on linux/arm. Thanks.

Actually, I found this by just reading source code. I have no accessible linux/arm here.
You are right. I should have tried it before filing as a issue. Sorry.
I'll try it later by QEMU or something. Thank you for your reply.

Input:
package main

import (
	"syscall"
)
func main() {
	t := new(syscall.Time_t)

	_, err := syscall.Time(t)
	if err != nil {
		println(err.Error())
	}
}

Output:
function not implemented

As I expected, linux return ENOSYS. is it enough info?

For the record, root of the problem is code-generation scripts couldn't recognize #undef.
We should fix such scripts and hope nothing changes after re-generation.

@hirochachacha the syscall package is a mismash of incompatible implementations. The go runtime does not use syscall.Time, it uses an implementation of nanotime(2), although that depends on the platform you are calling it on.
If there is a fix that needs to be made, it will be in the x/sys/unix package, the syscall package is closed for modifications.

the syscall package is a mismash of incompatible implementations. The go runtime does not use syscall.Time, it uses an implementation of nanotime(2), although that depends on the platform you are calling it on.

I see. And also runtime's getrlimit is implemented by ugetrlimit.
Yes, runtime's implementation looks like more sophisticated and well tested than syscall's one to me.

If there is a fix that needs to be made, it will be in the x/sys/unix package, the syscall package is closed for modifications.

Fair enough.
By the way, do you want to accept such a trivial bug report?
I know you guys work for more important issues everyday.
Maybe you don't want to spare time for trivial one.
Maybe you don't want to see the issue tracker is filled by trivial ones.
I don't want annoy you. That's not my intention.
So when I found a trivial issue, what should I do?
Thank you for responses, anyway.

Trivial or not, if it's a bug, we need to fix it.
If you think it's trivial, perhaps the best way
is to just send a CL to fix it :)
I think we need to fix x/sys/unix, and fix the syscall
package if the fix is simple. (IIUC, the syscall
package is frozen for non-bugfixes and additions
except adding of new ports, but if the implementation
of some existing functions is wrong, then we'd better
fix it.)
Reopen this issue.

Thanks! I'll send a CL.

and fix the syscall package if the fix is simple.

Yes it's simple. For example, we can implement Time by gettimeofday,
Utime by utimes and Getrlimit by ugetrlimit.
But corresponding syscall numbers are already declaraed by api/go*.txt.
(i.e. SYS_TIME, SYS_UMOUNT, etc ...)
How can I treat these?

@hirochachacha in general the syscall package exists to support the standard library. If the standard library does not need or use the function (even if it is buggy or not implemented), the preferred way is to fix it in the x/sys/unix repository.

@davecheney I'm confusing. which should I follow?

@hirochachacha we cannot change any of the public values declared in the syscall package. You may have to declare some private constants instead.
Also, please try to make as small as possible a fix. If the issue is not apparent to callers of this code, ie the rest of the standard library, the recommendation is to fix the issue in the x/sys/unix package instead.

Those SYS_* constants are still correct, it's just they
can't be used by the current kernel, so leave them as
they are. The syscall numbers are currently automatically
generated, so it probably hard to add comments to
those numbers, but I think most people will use existing
functions rather than calling Syscall directly anyway, so
it probably won't affect much.
Just fix the implementation to use the correct syscalls.
For the functions in syscall package, I think if the fix
is simple, we can fix those functions. The syscall package
is to serve the standard library, that's the reason why
we don't extend it anymore, but existing bugs should
still be fixed, IMO.

Can you please talk to each other? I want to follow the decision.

I am sorry for the confusion. I will let you and @minux decide what to do.
On Sat, Feb 27, 2016 at 8:10 PM, hirochachacha notifications@github.com
wrote:

Can you please talk to each other? I want to follow the decision.
—
Reply to this email directly or view it on GitHub
https://github.com/golang/go/issues/14524#issuecomment-189609571.

No worries. Thanks! I will follow @minux 's decision here.

To make my suggestions more concrete:

don't change constants in either syscall or x/sys/unix
(my understanding is that the numbers are correct but they
are not implemented by the kernel anymore. please correct
me if I'm wrong.) I'm also open to delete those constants in
x/sys/unix, but we have to keep those in the syscall package.

move affected functions from auto-generated zsyscall
file into a linux/arm specific file, and implement them correctly.

For x/sys/unix only, consider adding tests for syscalls that
are straightforward to test. For example, we can test time.Now()
and unix.Time agrees with each other. The test should be
generic so that it runs on all linux platforms (not only arm).

don't change constants in either syscall or x/sys/unix (my understanding is that the numbers are correct but they are not implemented by the kernel anymore. please correct me if I'm wrong.) I'm also open to delete those constants in x/sys/unix, but we have to keep those in the syscall package.

I investigated it. according to this,  EABI is supported since 2.6.16.
I downloaded linux-2.6.16.
Here is related source code:
include/asm-arm/unistd.h
#define __NR_time			(__NR_SYSCALL_BASE+ 13)

...

#if !defined(CONFIG_AEABI) || defined(CONFIG_OABI_COMPAT)
#define __ARCH_WANT_SYS_TIME
#define __ARCH_WANT_SYS_OLDUMOUNT
#define __ARCH_WANT_SYS_ALARM
#define __ARCH_WANT_SYS_UTIME
#define __ARCH_WANT_SYS_OLD_GETRLIMIT
#define __ARCH_WANT_OLD_READDIR
#define __ARCH_WANT_SYS_SOCKETCALL
#endif

kernel/time.c
#ifdef __ARCH_WANT_SYS_TIME

asmlinkage long sys_time(time_t __user * tloc)
{
...
}
...
#endif /* __ARCH_WANT_SYS_TIME */

arch/arm/kernel/calls.S
/* 0 */		CALL(sys_restart_syscall)
...
/* 10 */	CALL(sys_unlink)
		CALL(sys_execve_wrapper)
		CALL(sys_chdir)
		CALL(OBSOLETE(sys_time))	/* used by libc4 */
		CALL(sys_mknod)
/* 15 */	CALL(sys_chmod)
...

syscall table is defined by calls.S . So syscall numbers are defined, but functions are not.

move affected functions from auto-generated zsyscall file into a linux/arm specific file, and implement them correctly.

I see.

For x/sys/unix only, consider adding tests for syscalls that are straightforward to test. For example, we can test time.Now() and unix.Time agrees with each other. The test should be generic so that it runs on all linux platforms (not only arm).

I guess you mean time.Unix. I understand.
Thank you.

I guess you mean time.Unix

Sorry, I was wrong. Unix.Time is right.

For x/sys/unix only, consider adding tests for syscalls that are straightforward to test. For example, we can test time.Now() and unix.Time agrees with each other. The test should be generic so that it runs on all linux platforms (not only arm).

I guess you mean time.Unix. I understand.

I'm really sorry. I didn't understand it. what "agrees with" exactly mean?
Is this mean: abs(timeA - timeB) < some value ?

On Sat, Feb 27, 2016 at 6:24 AM, hirochachacha notifications@github.com
wrote:

For x/sys/unix only, consider adding tests for syscalls that are
straightforward to test. For example, we can test time.Now() and unix.Time
agrees with each other. The test should be generic so that it runs on all
linux platforms (not only arm).

I guess you mean time.Unix. I understand.
I'm really sorry. I didn't understand it. what "agrees with" exactly mean?
Is this mean: abs(timeA - timeB) < some value ?

Repeat ten times and see if timeA == timeB at least once?

Repeat ten times and see if timeA == timeB at least once?

Is this preferable to timeA - timeB < n seconds? and why?
Maybe both of them use CLOCK_REALTIME, so sometimes it failed? I'm not sure.

I was talking about the 1-second granularity Time() syscall.

I sent a CL for syscall package.
I've rethought about deleting obsolete syscall numbers, and don't want to change it now.
So if the CL is acceptable, then it's OK to close this.
Thank you.

The CL wasn't approved. But I suppose no one wants to continue this.
So I close it.
Thank you for responses.

authors: bradfitz davecheney hirochachacha minux


text:
hcigvjrjir

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
What operating system and processor architecture are you using (go env)?
What did you do?
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.
What did you expect to see?
What did you see instead?

hcigvjrjir

Damn, sorry, toddler.

No worries, at least it wasn't the cat.

authors: ianlancetaylor natefinch


text:
archive/tar: empty-value Xattrs are dropped with Reader

% go version
go version go1.8.3 linux/amd64
% go env
GOARCH="amd64"
GOBIN="/home/cyphar/go/bin"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/cyphar/.local"
GORACE=""
GOROOT="/usr/lib64/go/1.8"
GOTOOLDIR="/usr/lib64/go/1.8/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build495773064=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

You can verify this at https://play.golang.org/p/T9jR4DO492. Here's the contents of the paste:
package main

import (
	"archive/tar"
	"fmt"
	"io"
)

func main() {
	reader, writer := io.Pipe()

	tw := tar.NewWriter(writer)
	tr := tar.NewReader(reader)

	go func() {
		tw.WriteHeader(&tar.Header{
			Name: "somefile",
			Mode: 0777,
			Xattrs: map[string]string{
				"user.emptyvalue": "",
				"user.nonempty":   "some value",
			},
		})
		tw.Close()
	}()

	hdr, err := tr.Next()
	if err != nil {
		panic(err)
	}

	if value, ok := hdr.Xattrs["user.emptyvalue"]; !ok {
		fmt.Printf("expected user.emptyvalue to exist\n")
	} else if value != "" {
		fmt.Printf("expected user.emptyvalue to be '', got %q\n", value)
	}

	if value, ok := hdr.Xattrs["user.nonempty"]; !ok {
		fmt.Printf("expected user.nonempty to exist\n")
	} else if value != "some value" {
		fmt.Printf("expected user.nonempty to be 'some value', got %q\n", value)
	}
}

What did you expect to see?
No output.
What did you see instead?
expected user.emptyvalue to exist

This is happening on the reading, not writing side (which you can verify by dumping the generated tar archive to stdout). According this section of archive/tar it looks like this is intentional:
		default:
			// According to PAX specification, a value is stored only if it is
			// non-empty. Otherwise, the key is deleted.
			if len(value) > 0 {
				extHdrs[key] = value
			} else {
				delete(extHdrs, key)
			}
		}

Which begs the question, should we be generating xattrs that aren't valid according to PAX or should we accept such headers? I don't really mind either way, but it seems quite odd that we are liberal in what we generate but strict in what we accept.

/cc @dsnet

The PAX "specification" says the following:

If the  field is zero length, it shall delete any header
block field, previously entered extended header value, or
global extended header value of the same name.

Which means that zero-length values have a very specific meaning: deletion. So the correct solution is to prevent the tar.Writer from generating these bogus entries.
A lot of work happened on the tar.Reader in the Go1.8 cycle, and I was planning on vastly improving the writer in Go1.9, but really dropped the ball this cycle. Will attempt to get the fix in Go1.10.

authors: cyphar dsnet odeke-em


text:
proposal: spec: treat blank struct fields as having the same names for type identity

go version devel +c26aedbdde Fri Sep 15 11:28:52 2017 -0400 linux/amd64

Motivation
In #13467, I am attempting to use aliases of struct types to define mutually-assignable struct types in separate packages.
Because those struct types must match the layout of C structs, they use blank-named struct fields for padding and alignment. The spec mostly supports usage of blank-fields-as-padding (emphasis mine):

“Two struct values are equal if their corresponding non-blank fields are equal.”
“Within a struct, non-blank field names must be unique.”

The spec even gives an example of this padding use-case:
// A struct with 6 fields.
struct {
	x, y int
	u float32
	_ float32  // padding
	A *[]int
	F func()
}

Unfortunately, the rule for type identity does not respect the padding use-case (emphasis mine):
“Two struct types are identical if they have the same sequence of fields, and if corresponding fields have the same names, and identical types, and identical tags. Non-exported field names from different packages are always different.”
That implies that otherwise-identical structs with blank fields for alignment are treated as non-identical. For example, if two different packages define this type alias:
type S = struct {
	X byte
	_ [7]byte  // padding
	Y int32
}

it will result in types that are not mutually-assignable, despite sharing the same layout and accessible fields and producing otherwise-equal values.

Proposal
To remedy this problem, I propose a minor change to the rule for type identity:
“Two struct types are identical if they have the same sequence of fields, and if corresponding fields have the same names, and identical types, and identical tags. Non-exported, non-blank field names from different packages are always different.”

In practice, I expect this would be fine, but technically this is a backwards incompatible change.

Yes, it is technically backwards-incompatible. (In my mind, I consider it a “Specification error” for the purpose of Go 1 compatibility, but you may reasonably disagree.)

@bcmills I don't feel too strongly about it. Just @griesemer and I spent a while fixing #15514, so all the corner cases are still on my mind.

If you have
type Point { X, Y int }

then sometimes we write instead
type Point { X, Y int; _ struct{} }

to make sure that literals must be keyed and also that other packages can't convert between Point and a struct type literal written in another package (like struct { X, Y int }).
If we allow _ to all be identical then it does start allowing clients to depend on internal details in such structs again, although it's unclear why code would do that. (Literals would still have to be keyed.)
There's also a question of whether we want to encourage _ for padding any further. (We might want to guarantee layout only on an opt-in basis, for some kind of opt-in.)

to make sure that literals must be keyed

As you note, under this proposal those structs would still need to be keyed.
Allowing them to be unkeyed would also be more consistent with blank-fields-as-padding, but I don't currently have a compelling use-case for it.

and also that other packages can't convert between Point and a struct type literal written in another package (like struct { X, Y int }).

Note that under this proposal the type struct { X, Y int } would still be distinct, as it lacks the trailing field.
User code can already copy those structs explicitly (or by enumerating fields using reflection), so I'm not sure the lack of fields buys much.

There's also a question of whether we want to encourage _ for padding any further.

A blessed alternative for alignment and padding would also satisfy my use-case, as long as it does not interfere with type identity.

A blessed alternative for alignment and padding would also satisfy my use-case, as long as it does not interfere with type identity.

Can you clarify this? I would think it's desirable that
struct {
    X byte // offset 0
    Y byte // offset 1
}

and (something like)
struct {
     X byte // offset 0
//go:align 4
     Y byte // offset 4
}

be recognized as different types.
Otherwise, I think we'll need to complicate conversion rules: currently it's legal to convert *S to *T if S and T have identical underlying types, but we wouldn't want to allow that conversion if S and T's underlying types were the two struct types above.

@mdempsky, I agree that two types with different layouts or alignment should be treated as different types, even if they otherwise have the same exported fields. (IMO that's a compelling argument for not using //go: comments for alignment and padding: adding a comment to a type declaration should not affect its identity.)
What I mean by “does not interfere with type identity” is just that, whatever the mechanism, adding the same padding to a type used in two packages should not cause it to be treated as two distinct types.

Since padding and alignment are issues that only arise in the context of cgo, instead of changing the language with respect to all types, one could introduce types in package "C" and treat them specially in the compiler. E.g.
 struct {
      X byte // offset 0
      _ [4]C.align
      Y byte // offset 4
 }

For purposes of the language C.align would be a byte but the compiler "optimizes" the _ field down to 3 bytes. In the same vein you could have the compiler relax certain language rules for these C.... types without actually changing the language. I think this approach gives one the freedom to address the practical issues of interfacing with C code within Go1.
The only drawback I see is that a 3rd party compiler could be compliant with the Go1 language definition but be unable to compile a certain project using cgo properly. But with all the required external parts (C headers in particular) and platform dependencies any project using cgo is already fragile with respect to its platform and tools.

you could have the compiler relax certain language rules for these C.... types without actually changing the language.

A deviation from the language spec that is required in order for some common tool (such as cgo) to work is, de facto, a language change: it has nearly all the same disadvantages, plus the additional disadvantage of obscure documentation.

I think that this proposal is only for cases that arise when using cgo.  If we change cgo to name padding fields, rather than name them all _, then this proposal seems unnecessary.  That seems like a simpler change than modifying the language.

authors: bcmills ianlancetaylor mbenkmann mdempsky rsc