Add a new global method: Crafty.system()

It's very natural to want a non-entity object that can participate in the event system.  Currently when objects like the viewport or the graphical layers want to listen to an event, they have to refer to themselves via hardcoded references, since those event handlers get run with Crafty as the context.
This introduces the idea of systems which have a set of methods for participating in events.  These methods parallel those that Crafty and each entity implement.
When a system is created, a new object is created with these methods, and then it is extended with the methods/properties of a provided template object.  An init method and an events property can be provided that fill the same role as for components.  By default the system will be lazily created -- that is, it will only be created when referred to for the first time.  (This is what we do now with Crafty.canvasLayer and the like.)
In the future, we'll probably want to extend the capability of systems.  In particular, I envision that components and systems with the same name would have some connection -- perhaps the system could have methods for operating on all such entities at once.
I tested moving canvasLayer, domLayer, and webgl to this new paradigm and it seemed to work ok, but the actual switchover will happen in a new PR.
There were some small updates to the callbackMethods mixin required to implement this properly.

I like the idea very much. Conceptually it's like a singleton entity and component in one, which can be further composed of smaller systems. My thoughts:

as it's like a singleton entity and component combined, it should be familiar to and sport similar methods to Crafty.e  and Crafty.c - though this can be gradually added in future

in particular: remove-/add-/has-/require SubSystem, defineProperty(getter/setter), attr, timeout, getSystemId
also make the systems work with Model

continuing above's thought, I would like it to be able inherit multiple objects/systems like Crafty.e:

var physicsSystem = Crafty.system("PhysicsSystem", "CollisionSystem, GravitySystem"); - looks up systems by name
var physicsSystem = Crafty.system("PhysicsSystem", "CollisionSystem", "GravitySystem"); - alternative signature (altough this could be confusing)
var physicsSystem = Crafty.system("PhysicsSystem", collisionSystem, gravitySystem); - systems are given by object (although who has the other systems in scope when creating new system? :) )

what about a shorthand method instead to further stress the similarities to Crafty.e and Crafty.c -> Crafty.s(...)

Now onto implementation concerns:

Why is there no remove method invocation upon the system's destruction? The remove method of the template is called, which in my opinion should be handled by the template's destroy method itself - if the template needs to destruct itself, it itself should be a system and call it once destroyed
I would like to see a test case where a system extends a system

I should explain something about the intended usage.  The object that you pass in to Crafty.system isn't the system itself, but a kind of template for it.  (Much like how a component and an entity are different things.)  For instance, when canvasLayer is switched to this system:

Crafty will define an object, probably called just Crafty.canvasLayer for now.
On init, Crafty will call .system("Canvas", Crafty.canvasLayer).
This will help to allow multiple canvas layers; you could later call .system("CanvasUI", Crafty.canvasLayer), using the same object as the template.
It'll also help a bit if you stop and then reinit crafty -- all systems will be created anew from the template objects.

I would like it to be able inherit multiple objects/systems like Crafty.e

This isn't quite what I'm going for -- each thing labelled a systems should be complete in and of itself, so once you've defined a system you probably don't want to inherit from it.  There is the extend method, which will make it easy to share common functionality between systems through mixin objects.  Since one of the main goals of this whole setup is to abstract out how we handle render layers, that'll be a good test of how well it works.  You'll be able to create canvas/webgl/dom layers, but each will share a bunch of methods.

Why is the remove method called on the template object, rather than on this?

Initially I had that called destroy, rather than remove, which means it had to be called on the template.  I changed the name for consistency with how component methods are named, but didn't change the logic otherwise.

Ok, now I understand your use case a bit better.
So given your use-case:
// creating a system
var canvasLayer = { ... };
Crafty.system("Canvas", canvasLayer);
Crafty.system("CanvasUI", canvasLayer);
// retrieving a system
Crafty.system("Canvas");

what is the difference, advantages/disadvantages over
// creating a system
var canvasLayer = { ... };
Crafty.c("CanvasLayer", canvasLayer);
Crafty.e("Canvas", "CanvasLayer");
Crafty.e("CanvasUI", "CanvasLayer");
// retrieving a system
Crafty("Canvas").get(0);

It seems like Crafty already has the tools to achieve something like this (no need to reinvent the wheel), or am I missing something here? Perhaps you want to avoid looping over all entities in the selector?

It seems like Crafty already has the tools to achieve something like this (no need to reinvent the wheel), or am I missing something here? Perhaps you want to avoid looping over all entities in the selector?

There are a few reasons for adding a new type of object, rather than reusing components/entities.

Entities are initialized directly on creation, while we'll often want to init systems later on
I want each system to have its own name, which is a bit tricky to do with a collection of entities
Like I mention, I'd want systems to have some built in methods for handling collections of entities and other special interactions

It also just didn't feel right to me to make the viewport or graphical layers into entities.  It's probably technically possible, but I think it makes sense to make the distinction clear in the code.  They're different concepts, and they're going to be used in different ways.

It also just didn't feel right to me to make the viewport or graphical layers into entities

Ok, my main concern is to avoid code duplication. Now we will have e.g. Crafty.bind, Crafty.e.bind and Crafty.system.bind. It would be nice to abstract out this methods somehow (could be done in a later PR).
Other than that, looks good.

Ok, my main concern is to avoid code duplication. Now we will have e.g. Crafty.bind, Crafty.e.bind and Crafty.system.bind. It would be nice to abstract out this methods somehow (could be done in a later PR).

I actually tried to do this, but the functions really do require different logic in different contexts:

on the global Crafty object, trigger fires every callback, and in the future, I'd like for systems to trigger all the components they own (i.e. Crafty.system("Grid").trigger() works on all "Grid" components.)
All those functions can all be called on a group of entities (all the entities matching a selector) which requires logic for looping through them -- that doesn't make sense in the context of a system or the global Crafty.

What I ended up doing was having the underlying logic abstracted out -- that's what the _callbackMethod stuff is for -- then Crafty, entities and systems implement thin wrappers around that.

Other than that, looks good.

Thanks for the review, because these are definitely choices that should be well justified!
There are a couple of small changes I'll push (using Crafty.s instead of Crafty.system is a good idea).

I changed to Crafty.s and made it so that remove is called directly on the system, rather than from the template.

Great, I suggest adding defineProperty(getter/setter), attr, timeout, Model integration, construction example and retrieval example of systems in the documentation later down the road.

Can we merge this?

While working on a (failed, I got really sick last week) 7drl, I didn't see any huge issues with this, but there are a couple things that should be addressed.

I realized that you can't refer to a system inside the init function of that system, or you'll get stuck in an infinite loop.  I'll add a note to the documentation (don't create entities in init())
More importantly, there is some bug in the generic _callback code I need to sort out.  I think the problem crops up when you destroy an entity in response to an event when there are callbacks left.

Once I fix that bug we can merge this.

Fixed the bug (and added a test for it), and added the doc note.
I'll go ahead and merge this now.

 It's a very exhilarating experience to finish such a project successfully in a few days :)
I just played with event binding in my code, and had a programming error when binding to an event on the global Crafty instance.
Crafty.c("MyComponent", {
    init: function() {
        Crafty.bind("MyEvent", function() {
            this.myData = "example"; // this refers to Crafty here, not the entity that will have "MyComponent"
        });
    }
});

Was this always the case that this referred to Crafty when Crafty.bind was called and to the entity when entity.bind was called?
It's not a bug of course, because this behavior is clearly documented. I am just wondering if in an earlier Crafty version the this context was captured by the bind caller, so that the above code would have actually worked?

Was this always the case that this referred to Crafty when Crafty.bind was called and conversely to the entity when entity.bind was called?

I'm pretty sure -- we always triggered the callback with something like callback.call(context, data), which would override any previous call to bind.


Mean and std

When extractor features in c3d, Why should use mean and std like mean = [124 / 255, 117 / 255, 104 / 255]
std = [1 / (.0167 * 255)] * 3. This is not like imagenet mean and std

I used these statistics computed from the Kinetics dataset. Anyway, I consider training my own C3D model and recalculated this as well.


#Set the Format of the input text and the Snapshot.

The issue in detail is mentioned below with Screen Shots :
The Text entered is in this format :

But the text is not being accepted in the required format, instead it is being shown as a paragraph. :

In the Snapshot, it also being showed as a paragraph format instead as the original way of the input :

So go through all the 3 issues in detail and fix the issues with any related issues that are linked.

Hey
Could you assign it to me?

Hi @Badra00 The issue has been assigned to you. We are looking forward to your valuable contribution.


⚠ File access (beta) has degraded performance

In ccfc7a5, File access (beta) (https://beta.fyle.uk/file/fish) experienced degraded performance:

HTTP code: 200
Response time: 4961 ms

Resolved: File access (beta) performance has improved in b0e8b92.


[SR-9839] Fixes ambiguity in convention function argument inference

Resolves the ambiguity issue between @\convention function types where the solver finds two possibilities in this case () -> Void and @convention(block) () -> Void but couldn't disambiguate them.

Resolves SR-9839.
cc @xedin @hamishknight

Even better idea might be to join bindings based on representation if it's allowed, so instead of two bindings we'd just have one - @convention(block) () -> Void for the argument. See addPotentialBinding in https://github.com/apple/swift/blob/master/lib/Sema/CSBindings.cpp#L175. That's where such logic would go - https://github.com/apple/swift/blob/master/lib/AST/TypeJoinMeet.cpp#L298

Hey @xedin Thank you for the review :)
Increase the score on was the first approach I've attempted but couldn't quite make it work... I didn't know exactly how to identify the situation where to increase the score or not, so I thought it maybe simpler just adjust on ranking to choose one when comparing solutions ...

Even better idea might be to join bindings based on representation if it's allowed, so instead of two bindings we'd just have one - @convention(block) () -> Void for the argument. See addPotentialBinding in https://github.com/apple/swift/blob/master/lib/Sema/CSBindings.cpp#L175. That's where such logic would go - https://github.com/apple/swift/blob/master/lib/AST/TypeJoinMeet.cpp#L298

Awesome, I'm at work right now, so I'll take a look and change this later tonight :)

Sounds good!

I think it does make sense to join regular Swift convention to @convention(block) if type variable has to be a supertype of @convention(block) or any other convention of that matter.

@xedin Just a quick question, as I just got back to this ... this TypeJoin::visitFunctionType should be already been called because there is a supertype/subtype correlation between both potential bind types in this case? Or would we add logic specifically for functions checking supertype/subtype relation in addPotentialBindings and attempt Type::join if true? I've just placed a breakpoint into TypeJoin::visitFunctionType and it isn't called in this case ... Am I getting that right?

Hm, I guess it's not going to be called because one of the types is subtype and another is a supertype... I guess there is no other choice but to make it work with score in matchFunctionTypes...

Hm, I guess it's not going to be called because one of the types is subtype and another is a supertype... I guess there is no other choice but to make it work with score in matchFunctionTypes...

Humm ... that makes sense, but I'm still not sure how to go about to decide when to increase the scores or not, here are the matchings when solving the system for this case when both attempting type variable $T2 := () -> Void and attempting type variable $T2 := @convention(block) () -> Void
($T2 potentially_incomplete bindings={(supertypes of) () -> Void; (subtypes of) @convention(block) () -> Void})
  Initial bindings: $T2 := () -> Void, $T2 := @convention(block) () -> Void
  (attempting type variable $T2 := () -> Void
(lldb) p func1->dump()
(function_type escaping
  (input=function_params num_params=0)
  (output=type_alias_type decl=Swift.(file).Void underlying='()))
(lldb) p func2->dump()
(function_type representation=block escaping
  (input=function_params num_params=0)
  (output=type_alias_type decl=Swift.(file).Void underlying='()))
(lldb) p kind
(swift::constraints::ConstraintKind) $0 = ArgumentConversion
    (found solution 0 0 0 0 0 0 0 0 0 0 0 0)
  )
  (attempting type variable $T2 := @convention(block) () -> Void
(lldb) p func1->dump()
(function_type escaping
  (input=function_params num_params=0)
  (output=type_alias_type decl=Swift.(file).Void underlying='()))
(lldb) p func2->dump()
(function_type representation=block escaping
  (input=function_params num_params=0)
  (output=type_alias_type decl=Swift.(file).Void underlying='()))
(lldb) p kind
(swift::constraints::ConstraintKind) $1 = ArgumentConversion
(lldb)

So right now, I can't really see how I would increase the score to prefer an type over the other ...

Also, @xedin Thank you for the help here, really appreciate :)

That's curious since it doesn't matter what we attempt for $T2 it seems like we just end up with the same comparison, do you know why is that?

That's curious since it doesn't matter what we attempt for $T2 it seems like we just end up with the same comparison, do you know why is that?

No idea, looking into it right now :)

One thing I've noted is that I was expecting to get into the matchFunctionTypes for a Bind constraint, but seems that the bind constraint on TypeVariableBinding::attempt succeeds(without a matchFunctionTypes call) for and it tries to simplify() so then we got into matchingFunctionTypes by argument conversion constraints, which I think are:
Active Constraints:
  @lvalue () -> Void arg conv $T2 [[locator@0x11d01aa88 [Call@/Users/lucianoalmeida/Documents/Programming/swift-lang/swift/test/expr/closure/sr9839.swift:11:8 -> apply argument -> comparing call argument #0 to parameter #0]]];

Inactive Constraints:
  $T2 arg conv @convention(block) () -> Void [[locator@0x11d01ac18 [Call@/Users/lucianoalmeida/Documents/Programming/swift-lang/swift/test/expr/closure/sr9839.swift:11:1 -> apply argument -> comparing call argument #0 to parameter #0]]];

But even for the arg conv it still confusing to me ...

Bind would just assign a type for $T2 and argument conversions is when matchFunctionTypes would be used.  The more I think about this, the more it seems like a legit ambiguity because either type would require an implicit conversion, either for id or foo...

Seems like this also fixes SR-9840 :)

Bind would just assign a type for $T2 and argument conversions is when matchFunctionTypes would be used. The more I think about this, the more it seems like a legit ambiguity because either type would require an implicit conversion, either for id or foo...

Yeah, the problem is that the two types are both viable solutions(not sure if that's what you mean by "legit ambiguity") ... e.g.
func foo(_ x: @escaping @convention(block) () -> Void) {}
// If we ignore type inference, and the solver choose any of the solutions, they are both applicable
func f( _ a: @escaping () -> Void, _ b: @escaping @convention(block) () -> Void ) {
  foo(a) // Ok
  foo(b) // Ok
}

We can note this here:
Opened types:
  locator@0x11b00e250 [DeclRef@/Users/lucianoalmeida/Documents/Programming/swift-lang/swift/test/expr/closure/sr9839.swift:11:8] opens τ_0_0 -> $T2
  ($T2 potentially_incomplete bindings={(supertypes of) () -> Void; (subtypes of) @convention(block) () -> Void})
  Initial bindings: $T2 := () -> Void, $T2 := @convention(block) () -> Void
  (attempting type variable $T2 := () -> Void
    (found solution 0 0 0 0 0 0 0 0 0 0 0 0)
  )
  (attempting type variable $T2 := @convention(block) () -> Void
    (found solution 0 0 0 0 0 0 0 0 0 0 0 0)
  )

Sorry, I mean it's really ambiguous and not just a bug because @convention(block) () -> Void fits when implicitly converted to () -> Void for id and () -> Void fits when implicitly converted to @convention(block) () -> Void for foo, we can't really claim that either implicit conversion is better.
I think we should just make it diagnose ambiguous and be done.

Sorry, I mean it's really ambiguous and not just a bug because @convention(block) () -> Void fits when implicitly converted to () -> Void for id and () -> Void fits when implicitly converted to @convention(block) () -> Void for foo, we can't really claim that either implicit conversion is better.
I think we should just make it diagnose ambiguous and be done.

Humm, I see ... I thought that since both types could be implicit converted to one another what I called equivalent(not sure if that's the term here) it wouldn't really be a problem to just choose one of them :)
But right, I'll just revert the code and this will be just the test cases and we could be close this? :)

I think it would have to be more than just test-cases because at least original example from SR is not diagnosed as ambiguity at the moment...

I think it would have to be more than just test-cases because at least original example from SR is not diagnosed as ambiguity at the moment...

It is for SR-9839, ... right now here is the diagnostic
/Users/lucianoalmeida/Documents/Programming/swift-lang/swift/test/expr/closure/closures.swift:497:1: error: unexpected error produced: type of expression is ambiguous without more context
SR9839(id(qux))
^

Yeah, that's the problem, I think that diagnostic should point out exactly what is ambiguous here :)

Yeah, that's the problem, I think that diagnostic should point out exactly what is ambiguous here :)

Oh right, I'm on it :)

@xedin I was thinking in being more specific with those diagnostics and provide something like type ()->Void comes from argument conversion id(qux) and type ... comes from ... but I think that in order to do this we would have to track down the source locator for the type variable bindings in the constraint system.

I promise to take a look at this on Friday. Sorry, I got distracted by other things this week :/

@LucianoPAlmeida We could try one more way to disambiguate this. Similar to my suggestion to use join while inferring bindings, we could alternatively increase the score for @convention(block) binding in TypeVariableBinding::attempt if type variable represents a generic parameter and binding is a "subtype".
The difference between foo(p) and foo(id(qux)) is that there are no type variables involved in first example, all types are known upfront; but in the latter case id is a generic function so its generic parameter T has to be inferred based on constraints associated with it which is going to be two argument conversions:

from () -> Void to T; and (inferred from argument type passed to id)
from T to @convention(block) () -> Void (T is used a result type of id and passed as an argument to parameter of foo)

And there is going to be a solution for each type. That's why I was suggesting either try to join the types (which is unfortunately impossible) or increase a score for one of the cases.

I promise to take a look at this on Friday. Sorry, I got distracted by other things this week :/

No worries @xedin :) These things take time and you have other things 👍
As always, thank you for taking the time to look at this :))

Thanks for your help! It might just take a bit for me to get to this in queue...

Thanks for your help! It might just take a bit for me to get to this in queue...

No problem :)

@xedin this is ready for the review 👍
I know is in your queue ... and no worries, no rush to look at it :)
But meanwhile, can you or someone with rights kick the CI? Local tests are ok, but I'm just a bit curious if CI will pass as well...

@xedin Can you check on this one when you have some time? :) Just to know if this approach is ok and we just have to tweak details or should we go the other way and just diagnose this ambiguous ...

Sorry it took me awhile to come back to this, was trying to see whether I could come up with anything else given time... After looking at the code again I still feel like what we are trying to do is a hack and we should try to diagnose it better instead. WDYT?

Sorry it took me awhile to come back to this, was trying to see whether I could come up with anything else given time...

@xedin
No worries! Thanks for taking a look :)

After looking at the code again I still feel like what we are trying to do is a hack and we should try to diagnose it better instead. WDYT?

Agree, I kinda felt the same and also that this situation is too specific to worth making those hacks to only this specific case to work.
But great, let's do this then 👍
Thank you for the help :)

No worries, thank you for seeing it through!

Right, diagnostic implemented \o/
cc @xedin

Alright, let's give it a try.

@swift-ci please smoke test

\o/ Thanks @xedin!


Iris username registration

Hi.
I tried registering a username.
The captcha is confirmed, but then nothing happens and my account isn't registered.

I don't even get a captcha.  Is this part totally broken now?


Support for folder filter and nested image files

This is the new, consolidated PR.

@uweseimet After pondering this for a bit, I would propose that we set the default scan level to 1: the root dir and one level deep. For one, I think a new feature like this should be enabled by default, so that it gets noticed, used, and don't rot away.
I can't imagine very common usecases to have deeper directory nesting than this, however. Plus, there is the risk of overloading clients with huge amounts of data, in particular initially since the Web Interface is very unoptimized and calls SERVER_INFO very frequently (something that I have on my radar to improve in the future.)
Thoughts?

@rdmark In my use case, I will probably also use one level only, so scan level 1 would be sufficient for most use cases I guess.

The default is now a depth of 1.


Update

Problem solved
Short description of the bug fixed or feature added

PR-Codex overview
This PR refactors the ProposalButton component by replacing the Drawer with a Sheet for creating proposals. It enhances the form with validation and error handling using react-hook-form, and integrates analytics tracking for proposal creation events.
Detailed summary

Removed the Drawer component and replaced it with a Sheet component.
Introduced useState to manage the open state of the Sheet.
Added form handling using react-hook-form for a description input.
Included validation and error messages for the description field.
Integrated toast notifications for transaction success/error feedback.
Updated button and form structure for improved UI/UX.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

[!WARNING]
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

#4899  👈
#4898
#4885
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @kien-ngo and the rest of your teammates on  Graphite


Need to adjust the default Reply URL

In GitHub's Security tab you will be given the assertion consumer URL below the Digest choice.  That needs to be changed from the default in the Enterprise App config in AAD.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 156e3923-74d6-432f-c5a0-b905520d4e7d
Version Independent ID: aeca7d10-15e2-9e42-74bc-5309a5108c05
Content: Tutorial: Azure Active Directory integration with GitHub
Content Source: articles/active-directory/saas-apps/github-tutorial.md
Service: active-directory
Sub-service: saas-app-tutorial
GitHub Login: @jeevansd
Microsoft Alias: jeedes

@thejrack
Thanks for your feedback! We will investigate and update as appropriate.

Thanks @thejrack ! I made a PR to add a step for this. If you or @jeevansd think anything else should be added feel free to make any additions.

Step 4 in Azure AD SSO does not include instructions for the reply URL


On switch between Light/Dark modes keep user on the same tab

At the moment the user is taken to BALANCE tab on every change of the visual mode.

Already fixed.


Implementation of most of the API for Progress.

This implementation is based on the Darwin NSProgress implementation. One important detail is that swift-corelibs-foundation we do not have Key-Value Observing (KVO) yet. In the meantime, it is suggested to use polling or some other mechanism to read the values.
Updates along the parent/child relationship in Progress are working.
One additional thing that remains is an implementation of the localizedDescription methods. These are customized based on the contents of the userInfo dictionary.

@swift-ci please test

@swift-ci please test


Expected ')' error in Internet Explorer

I'm getting the error on this line in IE:
module.exports = function (options = {}) {
My guess it's cause default parameters are not allowed in IE.

I'm seeing the same issue since updating to version 2. Right now I'll have to downgrade back to version 1.3 until this is fixed

Seeing the same issue, downgrading back to 1.3.3 solved for now.

The package is not transpiled to ES5, you can add exception to your webpack/babel config.
I have used are-you-es5 package to get the babel-loader regex.


🚀[ Release v.2.9.0] Merge into Main

Overview
Maintenance to meet Google API Requirements

Stale PR....moving to v2.9.1


Error with vim.lsp.diagnostic.set_loclist() on InsertLeave - gopls

nvim --version: NVIM v0.5.0-31787a9
nvim-lsp version(commit hash): 76f11625481eddb7b
What language server(If the problem is related to a specific language server): gopls
Operating system/version: Ubuntu 18.04
Terminal name/version: Kitty
$TERM: xterm-kitty

How to reproduce the problem from neovim startup
init.vim
set runtimepath^=~/.local/share/nvim/site/pack/packer/opt/nvim-lspconfig

augroup UPDATE_DIAGNOSTICS_LOCLIST
	autocmd!
	autocmd InsertLeave * lua vim.lsp.diagnostic.set_loclist({open_loclist = false})
augroup END

lua << EOF

require'lspconfig'.gopls.setup{
	cmd = {"gopls", "serve"},
	settings = {
		gopls = {
			analyses = {
				unusedparams = true,
			},
			staticcheck = true,
		},
	},
}
EOF

main.go
package main

func main() {

	for {
}

Open main.go.
Go into insert mode then get back to normal mode, the following error is then triggered:

Error detected while processing InsertLeave Autocommands for "*":
E5108: Error executing lua ...t_nvimci4S1X/usr/share/nvim/runtime/lua/vim/lsp/util.lua:1357: Index out of bounds

How is this related to nvim-lspconfig? That error indicates there could be an issue with nvim itself.  nvim-lspconfig just provides the setup for starting an lsp for a given file type.